Rails core - Jul 2012 - sti_object.becomes(Parent) unexpectedly mutating the receiver

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<http://api.rubyonrails.org/classes/ActiveRecord/Persistence.html#method-i-becomes>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''

-- 
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/-/z8HmgZMRXgsJ.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

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

-- 
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 rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Just stumbled upon a pull request from last
year<https://github.com/rails/rails/pull/3023>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.

On Friday, July 20, 2012 6:45:31 PM UTC-4, Matt jones
wrote:
>
>
> 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 
>
>
-- 
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 rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

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


Dheeraj Kumar


On Saturday 21 July 2012 at 6:14 AM, Peter Brown wrote:
> Just stumbled upon a pull request from last year
(https://github.com/rails/rails/pull/3023) 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.
> 
> On Friday, July 20, 2012 6:45:31 PM UTC-4, Matt jones wrote:
> > 
> > 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 
> > 
> -- 
> 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 rubyonrails-core@googlegroups.com
(mailto:rubyonrails-core@googlegroups.com).
> To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
(mailto:rubyonrails-core+unsubscribe@googlegroups.com).
> 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 rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

-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


On Fri, Jul 20, 2012 at 11:22 PM, Dheeraj Kumar
<a.dheeraj.kumar@gmail.com> wrote:
> Got bit by this a couple of weeks ago. +1 for the merge.
>
>
> Dheeraj Kumar
>
> On Saturday 21 July 2012 at 6:14 AM, Peter Brown wrote:
>
> 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.
>
> On Friday, July 20, 2012 6:45:31 PM UTC-4, Matt jones wrote:
>
>
> 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
>
> --
> 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 rubyonrails-core@googlegroups.com.
> To unsubscribe from this group, send email to
> rubyonrails-core+unsubscribe@googlegroups.com.
> 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 rubyonrails-core@googlegroups.com.
> To unsubscribe from this group, send email to
> rubyonrails-core+unsubscribe@googlegroups.com.
> 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 rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

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 
>
>
> On Fri, Jul 20, 2012 at 11:22 PM, Dheeraj Kumar wrote: 
> > Got bit by this a couple of weeks ago. +1 for the merge. 
> > 
> > 
> > Dheeraj Kumar 
> > 
> > On Saturday 21 July 2012 at 6:14 AM, Peter Brown wrote: 
> > 
> > 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.
> > 
> > On Friday, July 20, 2012 6:45:31 PM UTC-4, Matt jones wrote: 
> > 
> > 
> > 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 
> > 
> > -- 
> > 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
> > 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 
> > 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 view this discussion on the web visit
https://groups.google.com/d/msg/rubyonrails-core/-/ThN68pR9-9UJ.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> 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.

-- 
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 rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

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?

-- 
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/-/qgoHS64wo8gJ.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

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.

-- 
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 rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.