-
Notifications
You must be signed in to change notification settings - Fork 829
Improve documentation on callables #4752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| <note> | ||
| <para> | ||
| If a callable of a protected or a private method is invoked from outside the class | ||
| with no visibility of said methods (e.g. being called from a POSIX signal handler), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A POSIX signal handler is a very niche example here. I know it's the one that prompted this PR, but I'd use something more common or self-evident.
| public function getProof() | ||
| { | ||
| return Closure::fromCallable([$this, 'hiddenWork']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using $this->hiddenWork(...) is preferred these days.
| <para> | ||
| <example> | ||
| <title> | ||
| Manually invoking callbacks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that the appropriate title here? The example feels a big long and involved, but I don't quite know what it's trying to demonstrate.
| <para> | ||
| If a callable of a protected or a private method is invoked from outside the class | ||
| with no visibility of said methods (e.g. being called from a POSIX signal handler), | ||
| a runtime error is thrown because the callable is invalid at the time of invocation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this means what you wanted to say. A callable or a closure of a private member can be invoked from outside the class. The problem is only with the pseudo-callables in the form of array which are converted into the actual closure at the invokation time only and therefore refer to an inaccessible member.
This page should probably be entirely rewritten to explain the modern concepts better.
| { | ||
| private function hiddenWork() | ||
| { | ||
| echo "256" . PHP_EOL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use PHP_EOL for displaying newline. This constant is for parsing text. Use "\n".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, what? No, PHP_EOL is entirely fine to use for output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It creates inconsistent output on different platforms and could confuse new users. The standard "\n" is shorter, unchangeing and universally understood by most programmers. We should avoid using PHP_EOL in the PHP manual.
|
@Vectorial1024 I have just created a PR #4933 which should supersede your PR. Can you please take a look whether this solves the problems you were trying to solve? You can suggest feedback on my PR if you'd like. |
|
@kamil-tekiela I do receive emails on comments of this PR, but often times I switched to look at something else and then forgot to come back to this. I'll take a look at the new PR. |
This implements #4355 :
The goal is that, by providing examples of how callbacks may be manually used, the reader becomes convinced that passing protected/private methods as callables to the outside world is generally a bad idea.