avram
avram copied to clipboard
renaming validate_numeric args for clarity.
Fixes #824
This is a breaking change. The idea here is that if you had this code:
number = attribute(4)
validate_numeric number, greater_than: 4
To me, this reads like if 4 > 4
which is false, so you might expect this to return false, but it does not. It returns true because it runs if 4 >= 4
. This especially gets weird if you're expecting it to be >
and you put something like
validate_numeric age, greater_than: 17
Which would allow age == 17
. The same is true on the upper end with less_than
. less_than: 100
still allows 100.
Now these are renamed to at_least
and no_more_than
so you can say
validate_numeric age, at_least: 18, no_more_than: 99
Sure, though, this next release will be filled with breaking changes already... I figured we might as well get as many in now as we can so that way moving to 1.0 there's none. But I'm ok with doing a deprecation here :+1:
Added in the deprecation. This is what it'll look like.
In spec/avram/validations_spec.cr:346:35
346 | result = Avram::Validations.validate_numeric(too_small_attribute, greater_than: 2)
^---------------
Warning: Deprecated Avram::Validations.validate_numeric:greater_than. Use validate_numeric with at_least/no_more_than instead of greater_than/less_than
Sorry to chime in late! I think at_least/no_more_than
is more accurate than the incorrect greater_than
, but may still not be the most clear.
What if we changed the name to exactly what it does using more "mathy" terms:
-
greater_than_or_equal_to
-
less_than_or_equal_to
And I'm not sure there is an option, but may be worth adding a greater_than
and less_than
that actually use >
and <
similar to https://guides.rubyonrails.org/active_record_validations.html#numericality
I think since these terms are fairly common and have a strictly defined meaning that may be more intuitive and accurate. It is more wordy for sure, but these types of validations typically aren't used that much, and if they are I think the wordiness is helpful to make sure it is accurate. What do you think?
Sure! This is why I did rc1 and not a full release :joy: get all our breaking changes out now so once 1.0 is solid, we don't break stuff. We can open a new issue for this