Skip to content
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

[Uid] make `Uuid::equals()` accept any types of argument for more flexibility #36058

Merged
merged 1 commit into from Mar 13, 2020

Conversation

@hhamon
Copy link
Contributor

hhamon commented Mar 13, 2020

Q A
Branch? master
Bug fix? no
New feature? no
Deprecations? no
Tickets ~
License MIT
Doc PR ~

I suggest to weaken the Uuid:equals method argument type to accept any types of value to compare against. This makes one able to compare the Uuid instance with any values.

@hhamon hhamon force-pushed the hhamon:uuid-equals-method branch from 2a1f8ed to 0c89aea Mar 13, 2020
@nicolas-grekas nicolas-grekas added this to the next milestone Mar 13, 2020
@lyrixx

This comment has been minimized.

Copy link
Member

lyrixx commented Mar 13, 2020

Why not, but I have an issue now:

$uuid = new Uuid(UUID_A);

$uuid->equald(new Uuid(UUID_A)); // <- this will return true
$uuid->equald(UUID_A); // But this will return false

Nothing in the API nor PHP doc tells about this behavior.

The question is:

What do we want?

It should be clarified somewhere

@nicolas-grekas nicolas-grekas changed the title [Uid] make `Uuid::equals` method accept any types of argument for more flexibility [Uid] make `Uuid::equals()` accept any types of argument for more flexibility Mar 13, 2020
@nicolas-grekas nicolas-grekas force-pushed the hhamon:uuid-equals-method branch from 0c89aea to 46721c1 Mar 13, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Mar 13, 2020

Thank you @hhamon.

@nicolas-grekas nicolas-grekas merged commit cc73b1e into symfony:master Mar 13, 2020
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details
*/
public function testEqualsAgainstOtherType($other)
{
$this->assertFalse((new Uuid(self::A_UUID_V4))->equals($other));

This comment has been minimized.

Copy link
@ro0NL

ro0NL Mar 13, 2020

Contributor

@hhamon what about sub classes? Should a different class with the same UUID value conceptually be considered equal?

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Mar 13, 2020

Member

from the pov of the Uuid class: yes
from the pov of the child class: theirs to decide, by overriding the equals() method if needed.
LSP :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.