sti_object.becomes(Parent) unexpectedly mutating the receiver

[Peter Brown]

I ran into an interesting issue today with ActiveRecord's becomes method and discovered that it is mutating the receiver without me knowing it.

The API docs say

"The new instance will share a link to the same attributes as the original class. So any change to the attributes in either instance will affect the other."

However, it doesn't say that the type attribute is changed on the receiver just by the method call.

Should the docs be updated to say that the receiver's type attribute will be changed to the class it becomes, or should becomes be changed so that it doesn't automatically mutate the receiver? Either option would be an easy fix, though the latter would break backwards compatibility. I am using becomes with things like form_for and content_tag_for so I'm using the new object returned by becomes as opposed to the mutated object.

Here's an example of the undocumented behavior I was seeing:

class Parent < ActiveRecord::Base
end

class Child < Parent
end

child = Child.new
child.type # => 'Child'
new_child = child.becomes(Parent)
child.type # => 'Parent'
new_child.type # => 'Parent'

[Matt Jones]

On Jul 20, 2012, at 5:48 PM, Peter Brown wrote:

> I ran into an interesting issue today with ActiveRecord's becomes method and discovered that it is mutating the receiver without me knowing it.
>
> The API docs say
>
> "The new instance will share a link to the same attributes as the original class. So any change to the attributes in either instance will affect the other."
>
> However, it doesn't say that the type attribute is changed on the receiver just by the method call.

'type' is an attribute; it gets changed - and the docs say the changes will happen to both. A bit unclear, but not a bug.

Definitely worth an update to the documentation, though.

--Matt Jones

[Peter Brown]

Just stumbled upon a pull request from last year with some discussion and it seemed like people were generally in favor of changing the behavior. I'd be willing to bring it back to life if people are still interested in it.

[Dheeraj Kumar]

Got bit by this a couple of weeks ago. +1 for the merge.

Dheeraj Kumar

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/22M16_XMSnYJ.
To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

[Nicolás Sanguinetti]

-1 for the merge.
+1 for a doc fix where this is more explicit.

I've always used x.becomes(Foo) for the side effect (mutating x)
instead of for the return value.

As I see it, the name _becomes_ clearly states that the receiver is
affected. If it was #convert or #cast or something that would
reasonably return a new object while not mutating the receiver I could
see how this could be thought of as a bug.

But it just makes sense that #becomes alters the original object.

Cheers,
-foca

[Peter Brown]

The reason I disagree with becomes mutating the object in place is because it's inconsistent with almost  all of Ruby and Rails' APIs. Methods that mutate an object (or raise an exception) end with !. Whether it's a string, hash, or array, methods that mutate these objects have two versions. There are exceptions such as String#clear, but compared to PHP, consistency is something Ruby and Rails have going for them.

Ruby:

• Array#map / map!
• Array#reverse / reverse!
• Array#reject / reject!
• Array#sort / sort!
• String#gsub / gsub 
• String#strip / strip!
• ...and the list goes on

Rails:

• Hash#symbolize_keys / symbolize_keys!
• Hash#slice / slice!

Why diverge from this pattern for a single case? If becomes is not the method for this, them maybe there should be an alternative like object.behave_like(Class)

On Saturday, July 21, 2012 3:48:08 PM UTC-4, Nicolás Sanguinetti wrote:

-1 for the merge.
+1 for a doc fix where this is more explicit.

I've always used x.becomes(Foo) for the side effect (mutating x)
instead of for the return value.

As I see it, the name _becomes_ clearly states that the receiver is
affected. If it was #convert or #cast or something that would
reasonably return a new object while not mutating the receiver I could
see how this could be thought of as a bug.

But it just makes sense that #becomes alters the original object.

Cheers,
-foca

> To unsubscribe from this group, send email to

> For more options, visit this group at
> http://groups.google.com/group/rubyonrails-core?hl=en.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To post to this group, send email to

> To unsubscribe from this group, send email to

[Steve Klabnik]

> Methods that
> mutate an object (or raise an exception) end with !.

This isn't correct. "dangerous" methods end with !. It has nothing to
do with mutation.

[Peter Brown]

I apologize, I wasn't trying to turn this into a debate about semantics. Steve, does that mean you're in favor of updating the docs too then? If so, any interest in adding a version that doesn't have side effects?

[Steve Klabnik]

I don't use this feature, so I don't have an opinion about the
non-side effect version, but I don't see how changing the docs to make
this behavior explicit would hurt anything.