On 17.03.21 09:36, Ilija Tovilo wrote:
Hi Marc
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.
Can you provide a concrete example where the current interface is
inadequate? I can't imagine a situation where you'd want to accept all
integer BackedEnums but string BackedEnums. If we ever expand the
types that backed enums accept the number of interfaces will grow
linearly.
/** @return class-string<BackedEnum> */
abstract public function getEnum(): string;
public function convertToDatabaseValue($phpValue, AbstractPlatform $platform): ?string {
$enum = $this->getEnum();
if (!$phpValue instanceof $enum) {
throw new RuntimeException('Unexpected $phpValue');
}
return (string)$phpValue->value;
}
// This currently requires reflection
public function convertToPHPValue($dbValue, AbstractPlatform $platform): BackedEnum {
$enum = $this->getEnum();
$refl = new ReflectionEnum($enum);
if ($refl->getBackingType()->__toString() === 'int') {
return $enum::from((int)$dbValue);
}
return $enum::from($dbValue);
}
// This would be cleaner and faster
public function convertToPHPValue($dbValue, AbstractPlatform $platform): IntEnum|StringEnum {
$enum = $this->getEnum();
if ($enum instanceof IntEnum) {
return $enum::from((int)$dbValue);
}
return $enum::from((string)$dbValue);
}
}
##########################
Another example would be an input filter
class EnumFilter extends Laminas\Filter\AbstractFilter {
/** @var class-string<BackedEnum> */
private string $enum;
public function filter($value) {
$enum = $this->enum;
$refl = new ReflectionEnum($enum);
if ($refl->getBackingType()->__toString() === 'int') {
return $enum::tryFrom((int)$value);
}
return $enum::tryFrom((string)$value);
}
// vs
public function filter($value) {
$enum = $this->enum;
if ($enum instanceof IntEnum) {
return $enum::tryFrom((int)$value);
}
return $enum::tryFrom((string)$value);
}
}
########################################
I can also think of static analyzers to detect type of from(), tryFrom() and $enum->value based on the implemented interface instead of requiring them to special case integer vs string backed enums.
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 ?
Historically, things like this have been done before if nobody
objects. But that seems unjustified at this point given the lack of
concrete reasoning.
Ilija
Thanks,
Marc