-
Notifications
You must be signed in to change notification settings - Fork 1
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
Min length rule #25
base: dev
Are you sure you want to change the base?
Min length rule #25
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, Mykhailo, see my comments
src/Rule/MaxLength.php
Outdated
@@ -13,9 +13,18 @@ class MaxLength implements ValidationRuleInterface | |||
*/ | |||
private $length; | |||
|
|||
public function __construct(int $length) | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not be part of the PR.
Although seeing, this it makes me think that a negative integer should not be passed to the function, but it is currently possible.
Could you make it throw an InvalidArgumentException in that case?
{ | ||
public function testValidate() | ||
{ | ||
$firstRule = new MaxLength(5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to my other comment with a negative value, it would require a test that makes sure the exception is thrown.
{ | ||
if($length<0) | ||
throw new InvalidArgumentException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When throwing an exception, a message should always be provided in order to inform the user why the exception was thrown without needing to look at the source code.
) | ||
{ | ||
if($minLength<0) | ||
throw new InvalidArgumentException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When throwing an exception, a message should always be provided in order to inform the user why the exception was thrown without needing to look at the source code.
*/ | ||
public function getMessage($v): string | ||
{ | ||
return $this->message?:"'${$v}' is supposed to be at least ".$this->minLength." characters long."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the message to:
"'The value '{$v}' was expected to be at least {$this->minLength} characters long."
No description provided.