-
Notifications
You must be signed in to change notification settings - Fork 45
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
Fixed the documentation. #50
base: master
Are you sure you want to change the base?
Conversation
This is needed in preparation for the PhpStorm stub (php-ds/ext-ds#128).
1. "All methods and properties of classes and interfaces must specify their visibility: public, protected or private.", from the PSR-12 draft. 2. It is required by convention for inclusion as a PhpStorm stub (see php-ds/ext-ds#128).
TravisCI failed because, apparently, the PHP Nightly is currently broken due to library dependency problems. |
Can I please get an update on the status of this PR? |
Thank you for the time and effort you have put into this PR @hopeseekr . ✨ |
|
||
/** | ||
* Determines if two objects should be considered equal. Both objects will | ||
* be instances of the same class but may not be the same instance. | ||
* | ||
* @param $obj An instance of the same class to compare to. | ||
* @param $obj self An instance of the same class to compare to. |
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 is correct - it could be anything at all. See php-ds/ext-ds#140
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.
Man, I am so confused.
I go down the same path as many of the commenters in that thread... First, I thought self
is correct, then object
, then, certainly ?object
, but then you're all saying that strings, ints, floats, what else? resources, arrays and callables should also be accepted?? I'm assuming Exceptions, too? lol.
I can understand ?object
, but just passing anything seems like a clear design flaw to me. But I'm very very unfamiliar w/ this project. I just wanted to do some good will and that conversation happened after I did the bulk of the changes.
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.
Man, I am so confused.
😁 no problem, I was for a while too. The thing here is that equals
is used to determine equality when an object is used as a key in a hash-based structure. ie. Map
will internally call an object's equals
method when looking up that object in the hash table.
The issue is that you can of course call equals
directly, ie. $map->equals(null)
. You can pass anything you want to that method, which means it is responsible for checking that the given arg is an instance of the same class etc. The doc comment should reflect that.
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.
Regardless of the correct type, it should precede the parameter name, not come after it.
This is needed in preparation for the PhpStorm stub (php-ds/ext-ds#128).