proposal: refactoring of options configuration

Signed-off-by: codisart <louis.celier@gmail.com>
laminas · May 9, 2020 · 208b1d7 · 208b1d7
1 parent 3f5e678
commit 208b1d7
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 30 deletions.
diff --git a/src/Between.php b/src/Between.php
@@ -76,17 +76,8 @@ public function __construct($options = null)
             $options = ArrayUtils::iteratorToArray($options);
         }
         if (! is_array($options)) {
-            $options = func_get_args();
-            $temp['min'] = array_shift($options);
-            if (! empty($options)) {
-                $temp['max'] = array_shift($options);
-            }
-
-            if (! empty($options)) {
-                $temp['inclusive'] = array_shift($options);
-            }
-
-            $options = $temp;
+            $validatorOptions = new ValidatorOptionsObject(['min', 'max', 'inclusive']);
+            $options = $validatorOptions->argumentsAsArray(func_get_args());
         }
 
         if (! array_key_exists('min', $options) || ! array_key_exists('max', $options)) {
@@ -110,6 +101,16 @@ public function __construct($options = null)
         parent::__construct($options);
     }
 
+    public function argumentsAsArray($args)
+    {
+        $keys = ['min' , 'max' , 'inclusive'];
+        $result = [];
+        foreach ($args as $arg) {
+            $result[array_shift($keys)] = $arg;
+        }
+        return $result;
+    }
+
     /**
      * Returns the min option
      *

diff --git a/src/StringLength.php b/src/StringLength.php
@@ -13,6 +13,8 @@
 
 class StringLength extends AbstractValidator
 {
+    use ValidatorOptionsTrait;
+
     const INVALID   = 'stringLengthInvalid';
     const TOO_SHORT = 'stringLengthTooShort';
     const TOO_LONG  = 'stringLengthTooLong';
@@ -52,17 +54,7 @@ class StringLength extends AbstractValidator
     public function __construct($options = [])
     {
         if (! is_array($options)) {
-            $options     = func_get_args();
-            $temp['min'] = array_shift($options);
-            if (! empty($options)) {
-                $temp['max'] = array_shift($options);
-            }
-
-            if (! empty($options)) {
-                $temp['encoding'] = array_shift($options);
-            }
-
-            $options = $temp;
+            $options = $this->argumentsAsArray(['min', 'max', 'encoding'], func_get_args());
         }
 
         parent::__construct($options);

diff --git a/src/ValidatorOptionsObject.php b/src/ValidatorOptionsObject.php
@@ -0,0 +1,34 @@
+<?php
+
+
+namespace Laminas\Validator;
+
+class ValidatorOptionsObject
+{
+    private $keys;
+
+    public function __construct(array $keys)
+    {
+        $this->shouldHaveOnlyStringValues($keys);
+        $this->keys = $keys;
+    }
+
+    private function shouldHaveOnlyStringValues(array $keys) : void
+    {
+        foreach ($keys as $key) {
+            if (! is_string($key)) {
+                throw new \InvalidArgumentException('The values of $keys should be only strings');
+            }
+        }
+    }
+
+    public function argumentsAsArray(array $args) : array
+    {
+        $keys = $this->keys;
+        $result = [];
+        foreach ($args as $arg) {
+            $result[array_shift($keys)] = $arg;
+        }
+        return $result;
+    }
+}
diff --git a/src/ValidatorOptionsTrait.php b/src/ValidatorOptionsTrait.php
@@ -0,0 +1,25 @@
+<?php
+
+namespace Laminas\Validator;
+
+trait ValidatorOptionsTrait
+{
+    public function argumentsAsArray(array $keys, array $args) : array
+    {
+        $this->shouldHaveOnlyStringValues($keys);
+        $result = [];
+        foreach ($args as $arg) {
+            $result[array_shift($keys)] = $arg;
+        }
+        return $result;
+    }
+
+    private function shouldHaveOnlyStringValues(array $keys) : void
+    {
+        foreach ($keys as $key) {
+            if (! is_string($key)) {
+                throw new \InvalidArgumentException('The values of $keys should be only strings');
+            }
+        }
+    }
+}
diff --git a/test/BetweenTest.php b/test/BetweenTest.php
@@ -258,19 +258,22 @@ public function testMissingMinOrMax(array $args)
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage("Missing option: 'min' and 'max' have to be given");
 
-        new Between($args);
+        new Between(...$args);
     }
 
     public function constructBetweenValidatorInvalidDataProvider()
     {
         return [
-            'only-min'      => [['min' => 1]],
-            'only-max'      => [['max' => 5]],
-            'min-inclusive' => [['min' => 0, 'inclusive' => true]],
-            'max-inclusive' => [['max' => 5, 'inclusive' => true]],
-            'min-undefined' => [['min' => 0, 'foo' => 'bar']],
-            'max-undefined' => [['max' => 5, 'foo' => 'bar']],
-            'no-min-or-max' => [['bar' => 'foo', 'foo' => 'bar']],
+            'array-only-min'      => [[['min' => 1]]],
+            'array-only-max'      => [[['max' => 5]]],
+            'array-min-inclusive' => [[['min' => 0, 'inclusive' => true]]],
+            'array-max-inclusive' => [[['max' => 5, 'inclusive' => true]]],
+            'array-min-undefined' => [[['min' => 0, 'foo' => 'bar']]],
+            'array-max-undefined' => [[['max' => 5, 'foo' => 'bar']]],
+            'array-no-min-or-max' => [[['bar' => 'foo', 'foo' => 'bar']]],
+
+            'arguments-only-min'      => [[1]],
+            'arguments-no-min-or-max' => [[]],
         ];
     }
 

diff --git a/test/GreaterThanTest.php b/test/GreaterThanTest.php
@@ -48,6 +48,30 @@ public function testBasic()
         }
     }
 
+    /**
+     * @covers \Laminas\Validator\GreaterThan::__construct()
+     * @dataProvider constructGreaterThanValidatorInvalidDataProvider
+     *
+     * @param array $args
+     */
+    public function testMissingMin(array $args)
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage("Missing option 'min'");
+
+        new GreaterThan(...$args);
+    }
+
+    public function constructGreaterThanValidatorInvalidDataProvider()
+    {
+        return [
+            'array-only-inclusive' => [[['inclusive' => true]]],
+            'array-inclusive' => [[['foo' => 5, 'inclusive' => true]]],
+            'array-only-fake' => [[['bar' => 'foo',]]],
+            'array-only-fake-2' => [[['bar' => 'foo', 'foo' => 'bar']]],
+        ];
+    }
+
     /**
      * Ensures that getMessages() returns expected default value
      *