Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new named constructor to create enum from mixed #135

Merged
merged 1 commit into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/Enum.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,24 @@ public function __construct($value)
$value = $value->getValue();
}

if (!$this->isValid($value)) {
throw new \UnexpectedValueException("Value '$value' is not part of the enum " . static::class);
}
static::assertValidValue($value);

/** @psalm-var T */
$this->value = $value;
}

/**
* @param mixed $value
* @return static
* @psalm-return static<T>
*/
public static function from($value): self
{
static::assertValidValue($value);

return new static($value);
}

/**
* @psalm-pure
* @return mixed
Expand Down Expand Up @@ -175,13 +185,27 @@ public static function toArray()
* @param $value
* @psalm-param mixed $value
* @psalm-pure
* @psalm-assert-if-true T $value
* @return bool
*/
public static function isValid($value)
{
return \in_array($value, static::toArray(), true);
}

/**
* Asserts valid enum value
*
* @psalm-pure
* @psalm-assert T $value
*/
public static function assertValidValue($value): void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to not extend the API also with this static method and have it private.
I wanted to change how it behaves here: #136 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drealecs the use-case for this method is to assert on input values from unknown contexts: the API is relatively minimal, and doesn't need to rely on the optimized structure you worked on in #136.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what you say. Yes, it can be useful as a public API as well to just validate without creating the instances.
It would have help a bit in making sure is_array or array_search are not called in constructor more than once when object is created,
is_array is already called twice when from() is used and I was thinking that should also be optimized.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this endpoint can be made completely independent from the ctor in the patch doing the perf optimization, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it can.

{
if (!static::isValid($value)) {
throw new \UnexpectedValueException("Value '$value' is not part of the enum " . static::class);
}
}

/**
* Check if is valid enum key
*
Expand Down
27 changes: 27 additions & 0 deletions tests/EnumTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ public function testCreatingEnumWithInvalidValue($value)
new EnumFixture($value);
}

/**
* @dataProvider invalidValueProvider
* @param mixed $value
*/
public function testFailToCreateEnumWithInvalidValueThroughNamedConstructor($value): void
{
$this->expectException(\UnexpectedValueException::class);
$this->expectExceptionMessage('is not part of the enum MyCLabs\Tests\Enum\EnumFixture');

EnumFixture::from($value);
}

/**
* Contains values not existing in EnumFixture
* @return array
Expand Down Expand Up @@ -332,4 +344,19 @@ public function testEnumValuesInheritance()
$inheritedEnumFixture = InheritedEnumFixture::VALUE();
new EnumFixture($inheritedEnumFixture);
}

/**
* @dataProvider isValidProvider
*/
public function testAssertValidValue($value, $isValid): void
{
if (!$isValid) {
$this->expectException(\UnexpectedValueException::class);
$this->expectExceptionMessage("Value '$value' is not part of the enum " . EnumFixture::class);
}

EnumFixture::assertValidValue($value);

self::assertTrue(EnumFixture::isValid($value));
}
}