On Wed, Dec 30, 2020, at 4:12 AM, Alexandru Pătrănescu wrote:

> Nice evolution overall.
> 
> Few notes:
> - I think ScalarEnum::fromValue() would be more clear than just from() and
> more in sync with ->value property.

Any name would work, I suppose.  I prefer short, single-word methods where 
possible, myself.

> - For the class structure you gave an example to illustrate the 'similar'
> way it is related to class, it would be nice to also mark the class as
> final.

It would.  Added.

> - I agree with no state behavior, at least at this point. However, I would
> have liked to offer the ability for very easy creation of
> singleton/multiton patterns using enums, like in Java.

I'm not sure what you're describing here.  Do you mean tagged unions/ADTs, 
which are punted to a later RFC?  

https://wiki.php.net/rfc/tagged_unions

> - Enums evolve in time and cases are added or sometimes removed. When the
> storage of the case stays externally (as scalar values or serialized),
> there could be issues when some of them are removed in the meantime.

Possibly; though that's no different than a class evolving or going away in 
time.  Either way it's up to the developer to figure out what BC strategy makes 
sense.

>   - for ScalarEnum::from(), for missing values I would guess we will throw
> an exception, maybe a specific one? Can we mention it in the RFC?

It already says a ValueError is thrown.  It probably does make sense to 
subclass that, though.  I'll mention it to Ilija.

>   - for deserialization, how should it be handled? using an exception as
> well, same exception as in the previous case.

Ilija and I discussed this a bit, and settled on an invalid deserialiation 
issuing a warning and returning false.  An exception would probably be better, 
but right now deserialize() doesn't throw things so introducing that just for 
enums seems inconsistent.  He'll be updating the implementation shortly.

> - The allowed_classes option on unserialize() method, I'm guessing it will
> work with Enums just like every other class, putting an instance of
> __PHP_Incomplete_Class?

Enums don't have unsafe behavior in their constructors the way classes do, so 
we're not convinced there's a reason to exclude them.  For now allowed_classes 
does not include enums.

> - Inheritance would work between enums. But only by eliminating some cases.
> I can see how enum RedSuites could extend enum Suites without breaking LSP.
> Not sure if it makes sense to dig on this now.
> 
> Regards,
> Alex

Possibly as a future scope, if we see demand for it.  The RFC is ambitious 
enough as is and already has several follow-ups planned for more advanced 
cases. :-)

Cheers!

--Larry Garfield

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to