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

Always return the same instance of a const #43

Closed
wants to merge 1 commit into from

Conversation

adrium
Copy link

@adrium adrium commented Jan 21, 2017

This enables:

function setAction(Action $action) {
    if ($action === Action::VIEW(); // ...
}

@mnapoli
Copy link
Member

mnapoli commented Jan 23, 2017

Hi, thank you for the proposal. I'm a bit wary to merge that because it would not solve all the problems. For example you could unserialize an enum, or simply do new MyEnum(MyEnum::FOO) (as I mentioned in #8) and get new values. So if we merge it it would be kind of an unreliable feature and it could be bad to advertise that enums are singletons.

I'd like to wait a bit to gather feedback and see if some people are interested by this and what they think of the possible downsides.

@adrium adrium force-pushed the same-instances branch 2 times, most recently from 5eca803 to 137d919 Compare January 30, 2017 18:17
@adrium
Copy link
Author

adrium commented Jan 30, 2017

#44 partially solves this by making the constructor private and register the instance, if it is unserialized.

@adrium adrium force-pushed the same-instances branch 2 times, most recently from 49cc7b8 to 117abc2 Compare August 28, 2017 20:06
@adrium adrium mentioned this pull request Aug 28, 2017
@shadowhand
Copy link

I oppose this for the reasons mentioned on #8. Enums are not entities and shouldn't be treated as singletons.

@mnapoli
Copy link
Member

mnapoli commented May 7, 2018

Given the comments above (and #8) let's close this for now.

@mnapoli mnapoli closed this May 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants