Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define a predicate method for boolean attributes #46376

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions activemodel/CHANGELOG.md
@@ -1,3 +1,23 @@
* Boolean attributes defined with the `ActiveModel::Attributes` API will now
have their respective predicate method defined automatically.

For example:

```ruby
class Person
include ActiveModel::Attributes

attribute :loves_rails, :boolean
end

person = Person.new
person.love_rails? # => false
person.love_rails = true
person.love_rails? # => true
```

*Connor McQuillan*

* Custom attribute types that inherit from Active Model built-in types and do
not override the `serialize` method will now benefit from an optimization
when serializing attribute values for the database.
Expand Down
5 changes: 5 additions & 0 deletions activemodel/lib/active_model/attribute_registration.rb
Expand Up @@ -16,6 +16,7 @@ def attribute(name, type = nil, default: (no_default = true), **options)
pending.type = type if type
pending.default = default unless no_default

define_predicate_method(name) if type.is_a?(ActiveModel::Type::Boolean)
reset_default_attributes
end

Expand Down Expand Up @@ -72,6 +73,10 @@ def resolve_attribute_name(name)
def resolve_type_name(name, **options)
Type.lookup(name, **options)
end

def define_predicate_method(name)
define_method("#{name}?") { public_send(name) == true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, == true part was added intentionally to make the method strictly return a boolean? (Never nil)

I think this is a reasonable design even though technically we have an option to drop this comparison as most of the values will be coerced to true or false and nil is falsey so it wouldn't be wrong.

But personally I'm leaning towards keeping the comparison to avoid situations like:
Person.new.loves_ruby? # => nil which from falseness perspective is "No" but in reality could mean "the person is not sure yet"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nvasilevski Can you think of a situation where we make this distinction? I was under the assumption that we care more about the semantics than the specific values 🤔

end
end
end
end
14 changes: 14 additions & 0 deletions activemodel/test/cases/attributes_test.rb
Expand Up @@ -115,6 +115,20 @@ def attribute=(_, _)
end
end

test "boolean attributes define predicate methods" do
model = ModelForAttributesTest.new(boolean_field: "1")

assert_equal true, model.boolean_field?

model.boolean_field = "false"

assert_equal false, model.boolean_field?

model.boolean_field = nil

assert_equal false, model.boolean_field?
end

test "children inherit attributes" do
data = ChildModelForAttributesTest.new(integer_field: "4.4")

Expand Down