gregnavis
Better handling of default values
Thank you for a lovely gem!
I have one issue so far: how the gem suggests I add presence validation to columns with default values. I use default values because I don't care about having a presence validation, which is how I thought everyone did it.
I still want to use that rake task but managing the ignores is cumbersome. Am I doing something wrong?
https://x.com/mhenrixon/status/1751157307566637102?s=20
I double-checked, and with default column values, rails do the expected thing. If I explicitly try to set the value to null, it blows up in the database.
This is perfectly fine for me, as I am trying to avoid defensive programming.
What do I mean by that?
Take the following
create_table "users", force: :cascade do |t|
t.integer "articles_count", default: 0, null: false
end
Why should I validate its presence? This is an internal Rails thing, and I want to go ahead and trust that the core Ruby on Rails programmers know how to ensure this is working correctly.
@mhenrixon, thank you for opening the issue.
As you said, we shouldn't be testing Rails implementation details, and that's not the goal of this validator.
In your example, it's still possible that the column would end up being set to nil, for example via:
User.create!(articles_count: nil)
That nil can come from return value, for instance. Additionally, for column types like string, the notion of being NOT NULL and present diverge considerably. A username of "" is NOT NULL but it's not present. In those cases a presence validator is even more necessary as that NOT NULL constraint captures only a part of the real requirement.
I personally provide user-facing default values in controllers, but I can see your approach working too.
For the reason explained above, I think, skipping columns with default values would be a mistake. However, I'm open to adding a setting to the validator like ignore_columns_with_defaults that would allow you to silence those warnings in one whole swoop.
How does that sound?
In your example, it's still possible that the column would end up being set to
nil,for example via:
It isn't possible because the column is not null in the database.
I'm happy with a setting to control this. Personally, I want to write as little code as possible.
Love the gem btw, phenomenal tool!
In your example, it's still possible that the column would end up being set to
nil,for example via:It isn't possible because the column is not null in the database.
What I meant was "a nil value may still be passed for insertion, and the default would not be applied (resulting in an exception)". Sorry for sloppy wording!
I'm happy with a setting to control this. Personally, I want to write as little code as possible.
Love the gem btw, phenomenal tool!
Thank you for your kind words! I'm happy you find it useful in your work!
What I meant was "a nil value may still be passed for insertion, and the default would not be applied (resulting in an exception)".
Yes, and while it isn't ideal, it is exceptional enough that I see it as wasteful to add a rails validation for that specific case.
Now I have to duplicate setting the default value in ruby, which is unnecessary.
Remember that not having the default in the database adds complexity to database operations on top of the additional default values and validations in Ruby on Rails.
In my opinion, this specific case (integer column with default value) is a false positive, and pursuing it adds unnecessary complexity.
I used to be super defensive in my programming, but lately, I see things differently.
You can blame it on studying the campfire code if you want.
It was liberating to see that much of the code I write wouldn't be written by the creators of the framework we all love to use.
It made me look at this differently.
After adding a presence validation, I could no longer rely on the default value and had to add a method to ensure that rails set the counter to zero on creation.
I thought about it and realized I'm not too fond of this suggestion. Hence, this issue.
I'm not trying to convince you of anything, @gregnavis, but I figured I should at least share how I ended up creating this issue.
I didn't address your comment about the string column. I agree that preventing an empty string is a good practice.
That said, a length validator might be more suitable here anyway, and validators often must be bypassed (update columns).
If we want to avoid an empty string, we should constrain that in the database regardless.
I appreciate the feedback and the discussion.
Another case just discovered - not null boolean columns with a default. Makes little sense to me having presence validations for these too.
