avram icon indicating copy to clipboard operation
avram copied to clipboard

renaming validate_numeric args for clarity.

Open jwoertink opened this issue 2 years ago • 1 comments

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

jwoertink avatar Aug 07 '22 17:08 jwoertink

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:

jwoertink avatar Aug 09 '22 15:08 jwoertink

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

jwoertink avatar Aug 21 '22 17:08 jwoertink

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?

paulcsmith avatar Sep 30 '22 14:09 paulcsmith

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

jwoertink avatar Sep 30 '22 15:09 jwoertink