Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112727
MIME-Version: 1.0
References: <eadd4a32-a05b-463b-a91f-a5d353c577b2@www.fastmail.com>
 <67c9d05a-f0a7-fcea-32ea-7a4e409308a1@mabe.berlin> <5120eee2-56c4-4608-9e41-4dd05563fef7@www.fastmail.com>
 <ae91f274-96bd-0041-43da-d5eb7a9a8986@mabe.berlin>
In-Reply-To: <ae91f274-96bd-0041-43da-d5eb7a9a8986@mabe.berlin>
Date: Sun, 3 Jan 2021 12:54:03 +0100
Message-ID: <CAPyj-LAkF_m8ZtnNTsxOSQh3B4zscevp-hmiCU-=1YBK35wcGg@mail.gmail.com>
To: php internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] [RFC] Enumerations, Round 2
From: tovilo.ilija@gmail.com (Ilija Tovilo)

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.

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.

> 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.

Ilija