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

Restore reverse lookup determinism #6097

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Jul 15, 2023

In #5036, it looks like I wrongly assumed I wouldn't be able to allow having 2 classes of the same type in the type registry without also allowing having 2 objects of the same type. array_search can perfectly tell the difference between both situations, so let us continue forbidding that last part, it will allow us to make sure a given type matches exactly one name.

In 96c2ebf, it looks like I wrongly
assumed I wouldn't be able to allow having 2 classes of the same type
in the type registry without also allowing having 2 objects of the same
type. array_search can perfectly tell the difference between both
situations, so let us continue forbidding that last part, it will allow
us to make sure a given type matches exactly one name.
derrabus
derrabus previously approved these changes Jul 15, 2023
@greg0ire
Copy link
Member Author

greg0ire commented Jul 15, 2023

@derrabus sorry, I forgot to implement override. Now that I look at it, this effectively reverts all the code introduced in #5036 🤔
This means that before that PR, the type registry was already not really using the flyweight pattern 😅
It was and is still possible to register the same class twice, just not the same object, and that object can have parameters.

@greg0ire greg0ire requested a review from derrabus July 17, 2023 15:39
@derrabus derrabus merged commit 54ae67d into doctrine:4.0.x Jul 19, 2023
@greg0ire greg0ire deleted the restore-lookup-determinism branch July 19, 2023 12:46
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants