Developed optional strict comparison for the IsInt validator.
#71
Conversation
src/Validator/IsInt.php
Outdated
| * | ||
| * @var int | ||
| */ | ||
| protected $strict = self::COMPARE_NOT_STRICT; |
There was a problem hiding this comment.
Why would this value be an int, rather than bool? I can't see there being any other values (strict vs not)
There was a problem hiding this comment.
Should I drop the constants if it'll always be a boolean?
src/Validator/IsInt.php
Outdated
| /** | ||
| * Sets the strict option mode | ||
| * | ||
| * @param $strict |
src/Validator/IsInt.php
Outdated
| */ | ||
| public function setStrict($strict) | ||
| { | ||
| if ($strict !== self::COMPARE_NOT_STRICT && $strict !== self::COMPARE_STRICT) { |
There was a problem hiding this comment.
If this is changed to be a bool then this check can be much simpler
src/Validator/IsInt.php
Outdated
| @@ -103,6 +144,9 @@ public function isValid($value) | |||
|
|
|||
| if (is_int($value)) { | |||
| return true; | |||
| } elseif (self::COMPARE_STRICT == $this->strict) { | |||
There was a problem hiding this comment.
Does this need to be an elseif here? Doesn't seem connected to the previous statement. Also, should be using a strict comparison (===), except if changed to bool as above, then it'd just be if ($this->strict) {
test/Validator/IsIntTest.php
Outdated
|
|
||
| /** | ||
| * @dataProvider strictIntDataProvider() | ||
| * @param $intVal |
test/Validator/IsIntTest.php
Outdated
| /** | ||
| * @dataProvider strictIntDataProvider() | ||
| * @param $intVal | ||
| * @param $expected |
test/Validator/IsIntTest.php
Outdated
| $this->validator->setStrict(-1); | ||
| } | ||
|
|
||
| public function strictIntDataProvider() |
There was a problem hiding this comment.
Docblock missing, at least we should have @return array (helps for adding PHP 7 return type declarations later)
src/Validator/IsInt.php
Outdated
| @@ -103,6 +144,9 @@ public function isValid($value) | |||
|
|
|||
| if (is_int($value)) { | |||
| return true; | |||
| } elseif (self::COMPARE_STRICT == $this->strict) { | |||
| $this->error(self::INVALID); | |||
There was a problem hiding this comment.
I wonder if we should introduce a new error type for this to ensure distinction (e.g. self::NOT_STRICTLY_INT for example)
src/Validator/IsInt.php
Outdated
| * Sets the strict option mode | ||
| * | ||
| * @param $strict | ||
| * @return $this |
There was a problem hiding this comment.
@return self (helps for adding return type declarations later)
src/Validator/IsInt.php
Outdated
| @@ -39,6 +42,13 @@ class IsInt extends AbstractValidator | |||
| protected $locale; | |||
|
|
|||
| /** | |||
| * Type of strict check to be used. "123" == 123 | |||
There was a problem hiding this comment.
I think comment could be expanded a little to explain what happens when "123" == 123 would be evaluated (i.e. say it would fail validation).
…pdated docblocks where necessary. Wrote docs for strict validation.
Fixes a CS issue as reported by phpcs. Additionally, updates the new tests to use `expectException()` instead of `setExpectedException()`.
|
Thanks, @wenkepaul! |
Strict comparison is off by default.