From ad1e07cf39fcc9a8313080cf4372e0fb26a24b7c Mon Sep 17 00:00:00 2001 From: Samuel Levy Date: Tue, 15 Mar 2022 07:24:25 +1000 Subject: [PATCH] Code review changes Dropped support for fragile indexes on pure enums --- README.md | 14 +++++++------- src/From.php | 24 +++++++++++++----------- tests/Pest/FromTest.php | 24 ++++++++++++------------ 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index e8eaaec..1398d41 100644 --- a/README.md +++ b/README.md @@ -151,7 +151,7 @@ This helper adds `from()` and `tryFrom()` to pure enums, and adds `fromName()` a #### Important Notes: * `BackedEnum` instances already implement their own `from()` and `tryFrom()` methods, which will not be overridden by this trait. Attempting to override those methods in a `BackedEnum` causes a fatal error. -* Pure enums do not actually have an internal index, so rearranging the order of cases will not break other code but it will create inconsistent behaviour with the `from()` and `tryFrom()` methods +* Pure enums only have named cases and not values, so the `from()` and `tryFrom()` methods are functionally equivalent to `fromName()` and `tryFromName()` #### Apply the trait on your enum ```php @@ -178,22 +178,22 @@ enum Role #### Use the `from()` method ```php -Role::from(0); // Role::ADMINISTRATOR -Role::from(5); // Error: \ValueError +Role::from('ADMINISTRATOR'); // Role::ADMINISTRATOR +Role::from('NOBODY'); // Error: ValueError ``` #### Use the `tryFrom()` method ```php -Role::tryFrom(2); // Role::GUEST -Role::tryFrom(5); // null +Role::tryFrom('GUEST'); // Role::GUEST +Role::tryFrom('NEVER'); // null ``` #### Use the `fromName()` method ```php TaskStatus::fromName('INCOMPLETE'); // TaskStatus::INCOMPLETE +TaskStatus::fromName('MISSING'); // Error: ValueError Role::fromName('SUBSCRIBER'); // Role::SUBSCRIBER -TaskStatus::fromName('MISSING'); // Error: \ValueError -Role::fromName('HACKER'); // Error: \ValueError +Role::fromName('HACKER'); // Error: ValueError ``` #### Use the `tryFromName()` method diff --git a/src/From.php b/src/From.php index 849780a..6a6a25f 100644 --- a/src/From.php +++ b/src/From.php @@ -4,44 +4,46 @@ declare(strict_types=1); namespace ArchTech\Enums; +use ValueError; + trait From { /** - * Gets the Enum by index for "Pure" enums. + * Gets the Enum by name, if it exists, for "Pure" enums. * * This will not override the `from()` method on BackedEnums * - * @throws \ValueError + * @throws ValueError */ - static public function from(int $case): static + public static function from(string $case): static { - return static::cases()[$case] ?? throw new \ValueError($case . ' is not a valid unit for enum "' . get_called_class() . '"'); + return static::fromName($case); } /** - * Gets the Enum by index, if it exists for "Pure" enums. + * Gets the Enum by name, if it exists, for "Pure" enums. * * This will not override the `tryFrom()` method on BackedEnums */ - static public function tryFrom(int $case): ?static + public static function tryFrom(string $case): ?static { - return static::cases()[$case] ?? null; + return static::tryFromName($case); } /** * Gets the Enum by name. * - * @throws \ValueError + * @throws ValueError */ - static public function fromName(string $case): static + public static function fromName(string $case): static { - return static::tryFromName($case) ?? throw new \ValueError('"' . $case . '" is not a valid name for enum "' . get_called_class() . '"'); + return static::tryFromName($case) ?? throw new ValueError('"' . $case . '" is not a valid name for enum "' . static::class . '"'); } /** * Gets the Enum by name, if it exists. */ - static public function tryFromName(string $case): ?static + public static function tryFromName(string $case): ?static { $cases = array_filter( static::cases(), diff --git a/tests/Pest/FromTest.php b/tests/Pest/FromTest.php index 49f07eb..f29e6c0 100644 --- a/tests/Pest/FromTest.php +++ b/tests/Pest/FromTest.php @@ -16,20 +16,20 @@ it('does not override the default BackedEnum tryFrom method with errors') ->expect(Status::tryFrom(2)) ->toBe(null); -it('can select a case by index with from() for pure enums') - ->expect(Role::from(0)) +it('can select a case by name with from() for pure enums') + ->expect(Role::from('ADMIN')) ->toBe(Role::ADMIN); -it('throws a value error when selecting a non-existent case by index with from() for pure enums', function () { - Role::from(2); -})->throws(\ValueError::class, '2 is not a valid unit for enum "Role"'); +it('throws a value error when selecting a non-existent case with from() for pure enums', function () { + Role::from('NOBODY'); +})->throws(\ValueError::class, '"NOBODY" is not a valid name for enum "Role"'); -it('can select a case by index with tryFrom() for pure enums') - ->expect(Role::tryFrom(1)) +it('can select a case by name with tryFrom() for pure enums') + ->expect(Role::tryFrom('GUEST')) ->toBe(Role::GUEST); -it('can returns null when selecting a non-existent case by index with tryFrom() for pure enums') - ->expect(Role::tryFrom(2)) +it('can returns null when selecting a non-existent case by name with tryFrom() for pure enums') + ->expect(Role::tryFrom('NOBODY')) ->toBe(null); it('can select a case by name with fromName() for pure enums') @@ -37,15 +37,15 @@ it('can select a case by name with fromName() for pure enums') ->toBe(Role::ADMIN); it('throws a value error when selecting a non-existent case by name with fromName() for pure enums', function () { - Role::fromName('NOTHING'); -})->throws(\ValueError::class, '"NOTHING" is not a valid name for enum "Role"'); + Role::fromName('NOBODY'); +})->throws(\ValueError::class, '"NOBODY" is not a valid name for enum "Role"'); it('can select a case by name with tryFromName() for pure enums') ->expect(Role::tryFromName('GUEST')) ->toBe(Role::GUEST); it('returns null when selecting a non-existent case by name with tryFromName() for pure enums') - ->expect(Role::tryFromName('NOTHING')) + ->expect(Role::tryFromName('NOBODY')) ->toBe(null); it('can select a case by name with fromName() for backed enums')