Re: [RFC] Enumerations, Round 2

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