Support traversables inside in operator
#113
Conversation
| @@ -89,7 +89,15 @@ public function __construct(Ruler\Context $context = null) | |||
| $this->setOperator('>=', function ($a, $b) { return $a >= $b; }); | |||
| $this->setOperator('<', function ($a, $b) { return $a < $b; }); | |||
| $this->setOperator('<=', function ($a, $b) { return $a <= $b; }); | |||
| $this->setOperator('in', function ($a, array $b) { return in_array($a, $b); }); | |||
| $this->setOperator('in', function ($a, $b) { | |||
There was a problem hiding this comment.
IMHO we still have to make a check:
if (false === is_array($b) && !$b instanceof \Traversable ) {
throw new \InvalidArgumentException(....
}
There was a problem hiding this comment.
Well similar error would be produced by PHP automatically during foreach call, no need to do it ourselves. But will do anyway.
| @@ -308,12 +308,18 @@ public function case_operator_in_empty_array() | |||
|
|
|||
| public function case_operator_in() | |||
| { | |||
| return $this->_case_boolean_operator('in', [7, [1, 3, 5, 7, 9]], true); | |||
| $this->_case_boolean_operator('in', [7, [1, 3, 5, 7, 9]], true); | |||
There was a problem hiding this comment.
This test case now contains 2 test cases. Please, split it :-).
There was a problem hiding this comment.
It's ok if test case contains multiple asserts. But ok will split.
| } | ||
|
|
||
| public function case_operator_in_falsy() | ||
| { | ||
| return $this->_case_boolean_operator('in', [42, [1, 3, 5, 7, 9]], false); | ||
| $this->_case_boolean_operator('in', [42, [1, 3, 5, 7, 9]], false); |
| $this->setOperator('in', function ($a, array $b) { return in_array($a, $b); }); | ||
| $this->setOperator('in', function ($a, $b) { | ||
| if (false === is_array($b) && !$b instanceof \Traversable) { | ||
| throw new \InvalidArgumentException(sprintf('Expect iterable, got %s', get_class($b))); |
There was a problem hiding this comment.
Please throw a Hoa\Ruler\Exception\Asserter instance.
| @@ -89,7 +89,19 @@ public function __construct(Ruler\Context $context = null) | |||
| $this->setOperator('>=', function ($a, $b) { return $a >= $b; }); | |||
| $this->setOperator('<', function ($a, $b) { return $a < $b; }); | |||
| $this->setOperator('<=', function ($a, $b) { return $a <= $b; }); | |||
| $this->setOperator('in', function ($a, array $b) { return in_array($a, $b); }); | |||
| $this->setOperator('in', function ($a, $b) { | |||
There was a problem hiding this comment.
Why not using the iterable type hint?
There was a problem hiding this comment.
We are dropping PHP 5.x support. So go on :-).
There was a problem hiding this comment.
Ok but iterable typehint is available since PHP 7.1 only. So can I drop all other versions from travis matrix?
|
Thanks for the PR! |
|
@Hywan No problem! So, how should we go forward with this? Do you want me to typehint to iterable and drop support for PHP < 7.1, or just change the type of exception thrown? |
|
@ostrolucky We are discussing about merging the |
|
All I see on gh is master branch, no php7? |
|
|
|
Or it's about to be :-p. |
|
Still no php7. Let's merge it as it is |
No description provided.