Conversation
src/validator-docs/Rules/Cpf.php
Outdated
|
|
||
| final class Cpf extends Sanitization | ||
| { | ||
| protected function validateFormat($value, $document, $attribute = null) |
There was a problem hiding this comment.
Moving this method from the Validator class isn't a good idea. There, it may be used to handle other document types like CNPJ or both.
You may inject it here and use it as you do.
There was a problem hiding this comment.
I am going to agree and disagree ...
I had it like that at first and it worked perfectly. BUT, when I looked at the code I saw that each validate (of the other types) was only 1 call, and it makes sense that each type of validation should do a thorough validation and validating the format is part of the validation. And that's why I moved it. Each of the validators should do a thorough validation .. that's my opinion. not leave it to an upper function to check formats as that is part of the validation.
There was a problem hiding this comment.
I have this all implemented on my side in our production APP and it's working every day perfectly .. ;-)
There was a problem hiding this comment.
Hmmm, good point! What do you think of isolating the method and changing the format validation to something like validateCpfFormat(string $value)? In this case, we may opt to move it to a similar context.
Please, apply the parameter's type hint to avoid errors in the future.
@geekcom any point more to add to this discussion?
2 participants
fixed the CPF validation
added a test for the fix