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

Provide a Faker Enum Provider Bridge #30

Merged
merged 5 commits into from
Jun 22, 2017
Merged

Conversation

xavier-rdo
Copy link
Contributor

@xavier-rdo xavier-rdo commented Jun 21, 2017

Fixes #29

  • Method enum(EnumAlias::VALUE) using pipes for flagged enums, e.g: enum(Permissions::WRITE|READ)
  • Method randomEnum(EnumAlias)
  • Unit tests for EnumProvider
  • Integration tests (loading Alice fixtures)
  • Update README, Integrations part

Copy link
Member

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this 😃


$reflectionEnum = new ReflectionClass($enumClass);

if (!$reflectionEnum->implementsInterface(EnumInterface::class)) {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than using reflection, you can use is_a with third argument set to true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanx for the tip !!!

*
* ```
* [
* 'civility' => Civility::class,
Copy link
Member

Choose a reason for hiding this comment

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

I'd use capitalized aliases: Civility

@xavier-rdo xavier-rdo force-pushed the feature/bridge-faker-provider branch 3 times, most recently from c5d4bb9 to c220f08 Compare June 21, 2017 15:20
}

/**
* @param string $enumValueShortcut As <ENUM_CLASS_ALIAS::ENUM_VALUE_CONSTANT>
Copy link
Member

Choose a reason for hiding this comment

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

< and > is specific to Alice. it should not appear here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh eh, < and > is just a convention for generic template masks but as a matter of fact, it can be ambiguous in an Alice/Fixture context.

/** @var EnumInterface $class */
$class = $this->enumMapping[$classAlias];

$constants = explode('|', $constants);
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should check if the enum class is an instance of FlaggedEnum to throw a nice exception otherwise

private function ensureEnumClass(string $enumClass)
{
if (!class_exists($enumClass)) {
throw new \InvalidArgumentException("$enumClass class does not exist");
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove this check as is_a below will already trigger autoload.

}

if (!is_a($enumClass, EnumInterface::class, true)) {
throw new \InvalidArgumentException("$enumClass is not a proper enum class");
Copy link
Member

Choose a reason for hiding this comment

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

You should use the one in the package (Elao\Enum\Exception\InvalidArgumentException)

@xavier-rdo xavier-rdo force-pushed the feature/bridge-faker-provider branch 6 times, most recently from d29c349 to d037be3 Compare June 22, 2017 07:33
<?php

/*
* This file is part of the Jarvis - Elao website.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jarvis ;( >> I will update my IDE config for this project

Copy link
Member

Choose a reason for hiding this comment

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

vendor/bin/php-cs-fixer fix

@xavier-rdo xavier-rdo force-pushed the feature/bridge-faker-provider branch from d037be3 to 5105219 Compare June 22, 2017 07:45
@xavier-rdo xavier-rdo force-pushed the feature/bridge-faker-provider branch from 86a627b to bc2542a Compare June 22, 2017 08:54
@xavier-rdo xavier-rdo force-pushed the feature/bridge-faker-provider branch 2 times, most recently from 7713f4f to ebf6a64 Compare June 22, 2017 09:31
$class = $this->enumMapping[$enumClassAlias];

$instances = $class::instances();
$randomRank = rand(0, count($instances) - 1);
Copy link
Member

Choose a reason for hiding this comment

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

Could you rather use mt_rand ?

$enumProvider = new EnumProvider($mapping);
}

public function provideErroneousEnumMapping(): array
Copy link
Member

Choose a reason for hiding this comment

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

diff --git a/tests/Unit/Bridge/Faker/Provider/EnumProviderTest.php b/tests/Unit/Bridge/Faker/Provider/EnumProviderTest.php
index f0640fc..dcfa153 100644
--- a/tests/Unit/Bridge/Faker/Provider/EnumProviderTest.php
+++ b/tests/Unit/Bridge/Faker/Provider/EnumProviderTest.php
@@ -50,25 +50,22 @@ class EnumProviderTest extends \PHPUnit_Framework_TestCase
      */
     public function testConstructorShouldFailWhenEnumClassIsIncorrect(array $mapping)
     {
-        $enumProvider = new EnumProvider($mapping);
+        new EnumProvider($mapping);
     }
 
-    public function provideErroneousEnumMapping(): array
+    public function provideErroneousEnumMapping()
     {
-        return [
+        yield 'EnumProvider constructor with a class that does not exist' => [
             [
-                // Pass to EnumProvider constructor a class that does not exist
-                [
-                    'Simple' => SimpleEnum::class,
-                    'Not-a-class' => '\UnexistingClass',
-                ],
+                'Simple' => SimpleEnum::class,
+                'Not-a-class' => '\UnexistingClass',
             ],
+        ];
+
+        yield 'EnumProvider constructor with a class that is not an Enum' => [
             [
-                // Pass to EnumProvider constructor a class that is not an Enum
-                [
-                    'Simple' => SimpleEnum::class,
-                    'Not-an-enum' => \DateTime::class,
-                ],
+                'Simple' => SimpleEnum::class,
+                'Not-an-enum' => \DateTime::class,
             ],
         ];
     }

Copy link
Member

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

Just a minor diff to apply, and I think we're good to go. Thank you :)

@xavier-rdo xavier-rdo force-pushed the feature/bridge-faker-provider branch from 1bc8b63 to 4e96d48 Compare June 22, 2017 10:59
Copy link
Contributor

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

👍 Nice

@chalasr chalasr merged commit 92bb0e6 into master Jun 22, 2017
@chalasr chalasr deleted the feature/bridge-faker-provider branch June 22, 2017 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alice Fixtures / Faker provider
3 participants