guillaumepotier / validator.js

Powerful objects and strings validation in javascript for Node and the browser
http://validatorjs.org
MIT License
255 stars 39 forks source link

Modify Length validator to match length exactly against a specified value #34

Closed hansondr closed 9 years ago

hansondr commented 9 years ago
ruimarinho commented 9 years ago

Have you considered using { min: 10, max: 10 } to get the exact length match?

hansondr commented 9 years ago

I have, and that is what I was doing previously, but it just felt a little awkward. This could be chalked up to personal preference but this is the first library I've used where 'length' accepted anything other than an integer.

ruimarinho commented 9 years ago

Maybe Range could be taught how to deal with "length range" instead? Both options would require a major bump though.

guillaumepotier commented 9 years ago

Overall this is great.

Just wondering if creating a Length and a LengthRange validator is not overkilled, though a very good practice to separate concerns/interface. Wondering if could not only affect Range validator, detect that we have a string/int only (and not an array) and set both this.min and this.max to the unique value given.

README.md documentation should be updated too.

But overall I repeat this is a great contrib and PR, much appreciated :)

Best

hansondr commented 9 years ago

Folding LengthRange into Range sounds like a good course of action. I've updated the PR with this modification.

ruimarinho commented 9 years ago

IMO, Range({ min: x, max: y }) is more readable than Range(x, y), so I'd still support the first argument type.

hansondr commented 9 years ago

I can definitely see your point @ruimarinho, I went with the existing Range parameters to minimize breaking changes but would be easy to change should @guillaumepotier decide to go in this direction.

guillaumepotier commented 9 years ago

I'm a bit confused I have to say. I come from a PHP Symfony world where Length validator works with min and max, on strings, and Range the same way, on numbers.

I think Length could support { min: min, max: max } AND integer. A Length(integer) validator only do not have much use IMHO.

On the other side, Range validator have always been for numbers (I think users are used to that) and using it for strings to support Length with min and max feels unnatural.

What do you think?

hansondr commented 9 years ago

All good points @guillaumepotier, I may have been over zealous in my attempt to solve my particular use-case in Parsley. (A length validation where the min and max value is the same yields an awkward error message, This value length is invalid. It should be between 10 and 10 characters long.) I ended up solving this issue by creating my own validator which may be the best solution. The other obvious alternative being a special case length error message when both the min & max are equal but that means additional localization

guillaumepotier commented 9 years ago

You're right. I'll definitely think of a way to address this issue when I have time. Did you use #31 to add your validator? I'm interested by how you did properly it in fact :)

Best