allow_blank option is meaningful?

Question

I'm trying to build own app in RoR.
How do you think below code.

class Book < ApplicationRecord
  validates :price,
    presence: true,
    inclusion: { in: 50..100, allow_blank: true }
end

price value must be in 50 to 100 and not empty.
My reviewer want to display "Price is not included in the list" and "Price can't be blank". But I don't agree with this comment.

My prefer code is below. This meets both inclusion and presence.

class Book < ApplicationRecord
  validates :price,
    inclusion: { in: 50..100 }
end

In this case, allow_blank is not meaningful.

How do you think? Which is more prefer?

UPDATE

Using numericality: is better for validating integer.

class Book < ApplicationRecord
  validates :price,
    numericality: { greater_than_or_equal_to: 50, less_than_or_equal_to: 100 }
end

score 3 · Answer 1 · answered Aug 08 '18 at 14:05

3

Either option is valid; you're trading succinctness of code against precision of the error message shown to the user. Which one works better can depend on the level of user who will be manipulating this record.

For a numeric value, however, consider a numericality: validation instead. "included in the list" is rather confusing phrasing even when a proper [but out of range] number is supplied.

(As an extra idea, consider submitting a PR to Rails to improve both of these default messages.)

answered Aug 08 '18 at 14:05

matthewd

4,332
15
21

In regards to your final comment. You always have full control over the error messages either via the `:message` `Hash` option or through [I18n locales](https://guides.rubyonrails.org/i18n.html#translations-for-active-record-models). [This SO Answer](https://stackoverflow.com/a/2859275/1978251) does a nice job of showing just how far you can take the localization of an error message – engineersmnky Aug 08 '18 at 15:35
@engineersmnky that doesn't mean we shouldn't make the defaults better – matthewd Aug 08 '18 at 15:38
What is wrong with the defaults? Defaults are meant to be generic and `in` could be `(1..1000)` or `('A'..'Z')` or `['foo','bar','baz']` or some homegrown object that implements `include?`. What generic message would make more sense than *"is not included in the list"* that would still apply unilaterally to all of these examples? – engineersmnky Aug 08 '18 at 15:41
I would likely merge patches that use "be blank" for blank values [that aren't in the list], or that use "in the range" over "list" where appropriate. – matthewd Aug 08 '18 at 15:46

allow_blank option is meaningful?

UPDATE

1 Answers1