Hi Ilija,
On 03.01.21 12:54, Ilija Tovilo wrote:
Hi Marc
I don't have a really good use-case for float values. It just seems
weird to me that a ScalarEnum doesn't support all scalars.
Using the enum value as array key for `cases()` works with your current
proposal but if we later want to allow floats, bool whatever then we got
a food gun.
The main reason is that we're using a hashmap internally in from() to
find the given case you're looking for. This is the same hashmap PHP
arrays are based on which only supports ints/strings as keys. If we
were to allow any scalar as a value, looking up a case by value would
become a O(n) operation.w
We could do something terrible like serialize the key before storing
it in the hashmap to allow arbitrary key types. But that will require
serializing the value on each invocation of from() which will
unnecessarily slow down the 95% most common use cases (int/string) to
support the exception. Note though that it's always easier to extend
than to remove. By not offering this feature we're erring on the side
of caution.
That being said, I can see how ScalarEnum is a misleading name. We've
been thinking about a better name and only had some ideas we weren't
fully satisfied with. RawEnum, ValueEnum and ConvertibleEnum were some
of these ideas. Let us know if you have a better suggestion.
That's reasonable and makes sense at this point. Only supporting string
and int is also fine (I don't have a personal use case for other types).
So using the same HT implementation as arrays internally totally makes
sense but this is an implementation detail not visible for the outside
and we shouldn't block outself for the future now as nobody knows of
unknown possible use cases. At least if we can avoid it.
You already provide a lookup mechanism with `MyEnum::from()` - I don't
see a real use-case for proving a pre build map. The main use case I see
is to list all possible enum values but this doesn't require a map and a
zero-indexed-array would also be more performant with packed arrays
(correct me if I'm wrong).
I do somewhat agree with you there. We're essentially returning
`Array<UnitEnum>|Map<int|string, ScalarEnum>` which feels
inconsistent. When you're calling cases() you're most likely going to
loop over it at which point $case->value is available at your
disposal.
Would you consider making `cases()` returning a simple list in all cases
instead of differentiate between UnitEnum and ScalarEnum given the fact
that mostly people just want to loop over cases and a lookup is already
available with ScalarEnum::from() to provide a cleaner interface?
Marc
Ilija
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
