Hi Ilija - thanks for your answers
On 14.03.21 23:18, Ilija Tovilo wrote:
Hi Marc
Thanks for testing the enum feature.
Hi I'm trying out the new enumeration support for PHP 8.1 using
https://github.com/php/php-src/pull/6489 thought to implement a polyfill
based on class constants and reflection. Doing so I come across the
newly defined interfaces:
``` interface UnitEnum{
public string$name;
public staticfunction cases(): array <http://www.php.net/array>;
}
interface BackedEnumextends UnitEnum{
public string$value;
public staticfunction from(int|string$scalar): static;
public staticfunction tryFrom(int|string$scalar): ?static;
}
``` My interest here is on BackedEnum which (currently) supports
int|string. First of all the interface defines a "public string $value"
but in fact the type needs to be "int|string". As properties are not
supported in interfaces this is a pure documentation issue in the RFC
itself.
We intentionally put it in the interface for clarity, even though it's
not technically possible yet.
Secondly the arguments name $scalar is not really a good name -
now as we have named arguments - $value would be much more meaningful.
I think you might be looking at an older implementation. I only fixed
the naming recently (~2 weeks ago). The parameter is called $value, as
you suggested.
nice +1
But most interestingly I get the feeling that this should be split into
several typed interfaces because this makes wrong impression especially
that "tryFrom" would except int|string but actually it doesn't.
Additionally, if we want to support other types later on this would blow
up the types even more. Example: ``` enum Test:string { case TEST = '1';
} Test::tryFrom('unknown'); // NULL Test::tryFrom(1);// TypeError ```
The TypeError doesn't make sense here as it matches the type and it even
matches the string representation of an existing type.
try/tryFrom will coerce the given value under non-strict_types. With
strict_types enabled it will type error. I think this behavior is
consistent with the rest of PHP.
I think it would
be much clearer if there would be two different interfaces like:
``` interface IntEnum extends UnitEnum { public int $value; public
static function from(int$value): static; public static function
tryFrom(int$value): ?static; }
interface StringEnum extends UnitEnum { public string $value; public
static function from(string $value): static; public static function
tryFrom(string $value): ?static; } ``` or else the above example should
return a NULL or even case matching the string representation instead of
TypeError as it matches the defined argument types.
I guess the interface would be more accurate. I'm not sure if that's a
huge win or just unnecessarily bloats the API. I think using
BackedEnum as a type hint should be relatively rare in practice.
Optimally we could add an associated type or generic type to
BackedEnum but that's currently not possible.
I think it would be at least be helpful for example on mapping an
enumeration from database to PHP as it's possible with doctrine types
and probably also for static analyzers without the need to special case
enumeration objects.
I also don't think one additional interface bloats up the API or waiting
for generics helps as nobody knows if this will ever be a thing in PHP.
Would this actually require a follow-up RFC or could this be done as a
small implementation detail change ?
Ilija
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php