Bugs in Facets Cloneable?

I was just looking at the Cloneable module from the Facets’ library,
because it’s useful, and noticed a couple of possible bugs. Is my
assessment of this correct?

module Cloneable
def clone
sibling = self.class.new
instance_variables.each do |ivar|
value = self.instance_variable_get(ivar)
sibling.instance_variable_set(ivar, value.dup) #rake_dup)
end
sibling
end
alias_method :dup, :clone
end

Question 1: Shouldn’t #clone use self.class.allocate instead of
self.class.new? Better yet, shouldn’t the superclass’s dup be called
somehow?

Question 2: Shouldn’t #clone and #dup be implemented differently, so
that
#clone can freeze the newly created object?

–Ken

On Jan 22, 11:09 am, Ken B. [email protected] wrote:

end

#clone can freeze the newly created object?
Hmmm… some good questions. This bit of code came from Rake a long
time ago. I never really analysed it, and just accepted it at face
value. I suspect the thing to do these days is to use #allocate and
#initialize_copy instead.

I point out, if anyone is wondering, the difference between this and
the built-in clone/dup is that the above dups the instance vars.

The difference between the two being that #dup carries over the
tainted state while #clone carries over the tainted and frozen states
(correct?). Since we are duplicating the instance vars here, there is
no reason to consider taintedness. But frozen should still apply
(yes?).

Could anyone else with a bit more assuredness about Ruby’s behavior
confirm this or offer the proper perspective if it is wrong?

Thanks,
T.

On Tue, 22 Jan 2008 17:14:55 -0500, Trans wrote:

  sibling.instance_variable_set(ivar, value.dup) #rake_dup)

Question 2: Shouldn’t #clone and #dup be implemented differently, so
The difference between the two being that #dup carries over the tainted
state while #clone carries over the tainted and frozen states
(correct?). Since we are duplicating the instance vars here, there is no
reason to consider taintedness. But frozen should still apply (yes?).

Basically, I think it should be possible for an object to encapsulate
(and hide) member variables that should be copied (not shared) when the
enclosing object is copied. Thus, a semi-deep copying version of #clone
and #dup are in order. That seems to be what Cloneable is about.

Thanks for pointing out #initialize_copy. I didn’t know it existed. Now
I
see it in PickAxe, but I think it’s role has been greatly trivialized.
Pickaxe only mentions it with regard to C extensions, empathetically
says
that it’s only for C extensions, and completely overlooks the idea that
an object might be hiding other kinds of information that should be
copied, not shared between duplicates.

Might I suggest that instead of overriding #dup and #clone at all,
Cloneable should just provide the following implementation for
#initialize_copy, then we can make #dup and #clone will both behave
properly by virtue of the fact that they descend from Object#dup and
Object#clone:

module Cloneable
def initialize_copy sibling
#first duplicate my superclass’ state. Note that if it’s duplicating
#instance variables, this will be overwritten, but this is important
#because we could be dealing with a C extension with state hidden
from
#the Ruby interpreter
super

#we want to know if we're being dup'ed or clone'd, because we want 

to
#preserve the state of our internals the same way our state is being
#preserved.
#(If we can’t figure it out, we’ll just use #dup)
operation=caller.find{|x| x !~ /initialize_copy'/}. match(/(dup|clone)’/)[1] or :dup

sibling.instance_variables.each do |ivar|
  value = sibling.instance_variable_get(ivar)

  #set my instance variable to be a #dup or #clone
  #or my sibling, depending on what's happening to me right now
  instance_variable_set(ivar, value.send(operation))
end

end
end

This will make the following test case work:

require ‘test/unit’

class Foo
include Cloneable
def initialize
@bar=[]
end
def bar_id
@bar.object_id
end
end

class TestCloneable < Test::Unit::TestCase
def test_dup
a=Foo.new
b=a.dup
assert_not_equal a.bar_id,b.bar_id

a.taint
b=a.dup
assert b.tainted?, "b should be tainted"

a.freeze
b=a.dup
assert !b.frozen?, "b should not be frozen"

end
def test_clone
a=Foo.new
b=a.clone
assert_not_equal a.bar_id,b.bar_id

a.taint
b=a.dup
assert b.tainted?, "b should be tainted"

a.freeze
b=a.clone
assert b.frozen?, "b should be frozen"

end
end

On Jan 22, 7:39 pm, Ken B. [email protected] wrote:

  value = self.instance_variable_get(ivar)

Thanks for pointing out #initialize_copy. I didn’t know it existed. Now I
Object#clone:

module Cloneable
def initialize_copy sibling
#first duplicate my superclass’ state. Note that if it’s duplicating
#instance variables, this will be overwritten, but this is important
#because we could be dealing with a C extension with state hidden from
#the Ruby interpreter
super

Nice. Makes sense to me. I will work on this a bit and incorporate –
with due credit.

Thanks Ken,
T.

On Jan 22, 7:39 pm, Ken B. [email protected] wrote:

copied, not shared between duplicates.
#instance variables, this will be overwritten, but this is important

sibling.instance_variables.each do |ivar|
  value = sibling.instance_variable_get(ivar)

  #set my instance variable to be a #dup or #clone
  #or my sibling, depending on what's happening to me right now
  instance_variable_set(ivar, value.send(operation))
end

end
end

Thanks for this Ken. I’ve adopted this code as given --I could not
think of better way to determine if dup or clone was being used. If
anyone knows of a more robust way to determine dup vs. clone please
let me know.

Thanks,
T.

On Feb 6, 2008, at 8:52 AM, Trans wrote:

Thanks for pointing out #initialize_copy. I didn’t know it existed.
Now I
see it in PickAxe, but I think it’s role has been greatly
trivialized.
Pickaxe only mentions it with regard to C extensions,
empathetically says
that it’s only for C extensions, and completely overlooks the idea
that
an object might be hiding other kinds of information that should be
copied, not shared between duplicates.

Yes, that was an oversight, and one that I’ll fix in the new edition.

Dave