rails-style-guide
rails-style-guide copied to clipboard
Clarify why model[] is preferred over read_attribute
Addresses https://github.com/rubocop/rails-style-guide/issues/155
@pirj that's a good point, I can adapt the PR to take this into account. How about this?
Prefer
self[:attribute]overread_attribute(:attribute), if the latter is used without a block.self[]raises anActiveModel::MissingAttributeErrorif the attribute is missing, whereasread_attributedoes not.[source,ruby] ---- # bad def amount read_attribute(:amount) * 100 end # good def amount self[:amount] * 100 end def amount read_attribute(:amount) { 0 } * 100 end ----
I think this is unnecessary, the PR is good as is.
The point in preferring [] is to fail during the development phase making typos in attribute names obvious. I can't think of a real-world usage of a useful read_attribute fallback when the attribute doesn't really exist on the model.
After looking more into the Rails code, I found out the conclusion from the referenced discussion is not exactly correct.
If I just try any random attribute with [], I don't get an exception raised:
model[:wrong_attr]
# => nil
The MissingAttributeError exception comes in when an attribute exists in the model, but is not initialized, as written in the comments before the method definition:
person = Person.select('id').first
person[:name] # => ActiveModel::MissingAttributeError: missing attribute: name
The important thing to realize is that the exception is only raised when Person has a name column. Otherwise the result would be nil. Given that, I'd say the text I introduced in this PR is misleading.