remi / her

Her is an ORM (Object Relational Mapper) that maps REST resources to Ruby objects. It is designed to build applications that are powered by a RESTful API instead of a database.
MIT License
2.05k stars 322 forks source link

`dup` and `clone` doesn't work on Her::Models #355

Open RaVbaker opened 9 years ago

RaVbaker commented 9 years ago

I tried to run a dup or clone on instance of Her::Model but it doesn't work and attributes are shared between copies of that instance. I believe it should work differently and the internal state of object should be copied on clone.

$ irb
2.0.0-p247 :001 > require 'her'
 => true
2.0.0-p247 :002 > class HerModel; include Her::Model;  end
 => HerModel
2.0.0-p247 :003 > i1 = HerModel.new(foo: 42)
 => #<HerModel(her_models) foo=42>
2.0.0-p247 :004 > i2 = i1.clone
 => #<HerModel(her_models) foo=42>
2.0.0-p247 :005 > i2.foo = 7
 => 7
2.0.0-p247 :006 > i2
 => #<HerModel(her_models) foo=7>
2.0.0-p247 :007 > i1
 => #<HerModel(her_models) foo=7>
2.0.0-p247 :08 > i1.object_id
 => 70162419040340
2.0.0-p247 :09 > i2.object_id
 => 70162431153300
hubert commented 9 years ago

are you sure the behavior you are getting is the same for dup and clone? i'm fairly sure that Her is relying on the underlying ruby implementation of these methods and the example you show above is consistent with the clone behavior of ruby objects.

RaVbaker commented 9 years ago

Yes, it's broken. dup is also doing it wrong.

$ irb
2.0.0-p645 :001 > require 'her'
 => true
2.0.0-p645 :002 > class HerModel; include Her::Model;  end
 => HerModel
2.0.0-p645 :003 > i1 = HerModel.new(foo: 42)
 => #<HerModel(her_models) foo=42>
2.0.0-p645 :004 > i2 = i1.dup
 => #<HerModel(her_models) foo=42>
2.0.0-p645 :005 > i2.foo = 7
 => 7
2.0.0-p645 :006 > i2
 => #<HerModel(her_models) foo=7>
2.0.0-p645 :007 > i1
 => #<HerModel(her_models) foo=7>
2.0.0-p645 :008 > i1.object_id
 => 70109072228660
2.0.0-p645 :009 > i2.object_id
 => 70109072277660
RaVbaker commented 9 years ago

As you see changing attribute of one of cloned objects also changes it for the other one.

bronson commented 8 years ago

You don't want clone: http://api.rubyonrails.org/v4.0.8/classes/ActiveRecord/Core.html#method-i-clone

Using dup works as you're describing in AR. Maybe Her should supply its own dup implementation? Like AR. Sounds like a good opportunity for a PR.

Here's more info: https://yacodeblog.wordpress.com/2012/08/08/adventures-in-cloning-activerecord-instances/