Browse files

attributes_before_type_cast are just the value of @attributes

  • Loading branch information...
1 parent 896e25e commit 3e842559427746eb9051ce5ee3cd36caeff303a3 @spastorino spastorino committed Dec 3, 2010
Showing with 1 addition and 1 deletion.
  1. +1 −1 activerecord/lib/active_record/attribute_methods/before_type_cast.rb
View
2 activerecord/lib/active_record/attribute_methods/before_type_cast.rb
@@ -13,7 +13,7 @@ def read_attribute_before_type_cast(attr_name)
# Returns a hash of attributes before typecasting and deserialization.
def attributes_before_type_cast
- Hash[attribute_names.map { |name| [name, read_attribute_before_type_cast(name)] }]
+ @attributes
end
private

9 comments on commit 3e84255

@pahanix

I think @attributes.dup is better here to avoid changing them by accident

@spastorino
Ruby on Rails member

@pahanix please investigate a bit more applying your patch I get 4 failures

@pahanix

I think this commit should be reverted because even for @attributes.dup attributes_before_type_cast["title"].replace("bye bye original string") does affect @attributes hash

My new patch is here http://pastie.org/1348840

@josevalim
Ruby on Rails member

Not sure if I agree with attributes.dup and cloning each value. What if I really want to change the attributes value or insert something new in the hash? If you accidentally change it, your tests should catch it. Ruby/Rails philosophy is not to write defensive code to protect you from eventual mistakes.

@pahanix

if you want to change @attributes hash you can access it by instance variable. But providing access to private variable through public method it is not good idea. Or you should document that attributes_before_type_cast it the way you access @attributes

@josevalim
Ruby on Rails member

I disagree. @attributes is internal to rails, and you should not be touching it from your app or plugins. Instead, you should use whatever public API rails offers. Which in this case can be the method under discussion here. This allows us to change the implementation at will.

@josevalim
Ruby on Rails member

Anyway, it seems we should dup @attributes for backwards compatibility. But -1 for dupping each value in it.

@pahanix

I support idea to have a sepate public api method to access @attributes, because for now is more readable to touch @attribues directly then through #attributes_before_typecast

Please sign in to comment.