Open
Description
I'm not quite sure why it's necessary for booleans to have a default value. Obviously when you are adding a boolean column to an existing table it may be required, because adding non-null columns requires a default in most cases. A newly created table however doesn't have a technical need for a default value for non-null tables, and it seems entirely plausible for a column not to have a good default value (whether it's an integer/string or as in this case a boolean).
Metadata
Metadata
Assignees
Labels
No labels
Activity
pirj commentedon Aug 29, 2023
Have you had a chance to check #275 #337 #43?
joerixaop commentedon Aug 30, 2023
#43 is about where to add a default if you want to add one. I don't want to add one, but I do think a lint about booleans not being nullable is sensible, adding a requirement for a default value in that same lint makes it difficult to use when you do not want a default value .
#275 doesn't seem to have any discussion about why requiring a default would be part of the lint that requires a boolean to be not-null. The linked blog post points out what I also pointed out: a default is required when adding a column to an existing table with existing rows, that is guaranteed to be not the case when you are adding a boolean column with create_table.
I don't really understand what #337 has to do with default values (boolean or otherwise).
pirj commentedon Sep 2, 2023
If you would decide to send a PR, what would it add/remove/change?
armandmgt commentedon Jan 13, 2025
Jumping in because I recently faced this:
We could enforce the default only when using
add_column
orchange_table
. When creating a new table only the nullability should be checked.In terms of good/bad examples we could make the following change:
Would that make sense @pirj ?
pirj commentedon Jan 13, 2025
I didn't get, why we can only enforce the default when changing an existing table. Can you please explain, @armandmgt ?
armandmgt commentedon Jan 13, 2025
In the thoughtbot article used as reference for this cop, they suggest to add the default because otherwise the migration would fail for existing rows.
When creating a new table the default value is not necessary, and sometimes it does not make sense (e.g. a Payments table with a column
tax_exemption
–tax_exemption
can be equally likely true or false)pirj commentedon Jan 13, 2025
tax_exemption is either true or false, it shouldn’t be null, or can it?
But now I see what you mean.
The point is indeed not to enforce the presence of a default value, the point is to prevent nulls.
Is this what you had in mind?
armandmgt commentedon Jan 14, 2025
Exactly, the tax_exemption column would still have the NOT NULL constraint 👍
And yes, my interpretation of the thoughtbot article is that we should enforce the NOT NULL constraint. The default values are a secondary matter, relevant only for existing tables
pirj commentedon Jan 14, 2025
@koic @andyw8 Without diving too fare into the discussion, can you please check the last few messages.
Does the suggestion to change the 3-state boolean guideline to have emphasis on being NOT NULL instead of enforcing a default value sound like a reasonable change to you?
cc @tejasbubane just in case, this was your original contribution to the style guide.
koic commentedon Jan 14, 2025
Yeah, the sequence of first setting
null: false
and then specifying a default value of eithertrue
orfalse
seems appropriate, IMO.In other words, focusing on preventing three-state values by ensuring that
boolean
columns havenull: false
as the top priority makes sense.pirj commentedon Jan 14, 2025
@armandmgt would you like to send a PR?
armandmgt commentedon Jan 14, 2025
@pirj Yes ! I opened one here #363 🙂