bloom-lang / bud

Prototype Bud runtime (Bloom Under Development)
http://bloom-lang.net
Other
854 stars 60 forks source link

Remove monkeypatch of Array#== #266

Closed neilconway closed 11 years ago

neilconway commented 12 years ago

Right now, we monkeypatch Array#== so that array literals and Bud tuples can be compared for equality. Monkeypatching such a fundamental method is a bit unfortunate (as the comment in monkeypatch.rb notes); it also has a performance cost. If the only thing this is useful for is the test cases, that should be easy to fix (e.g., add an assert_tuple_equal method to MiniTest). On the other hand, we've claimed in various places that Bud tuples are arrays, so not allowing tuples and arrays to be compared for equality might be unexpected.

neilconway commented 12 years ago

Note that if we do keep the current Array monkeypatch, the current implementation needs improvement:

1.9.3-p125 :001 > class Array
1.9.3-p125 :002?>     alias :oldeq :==
1.9.3-p125 :003 >       def ==(o)
1.9.3-p125 :004?>         if o.kind_of? Struct
1.9.3-p125 :005?>             o = (o.to_a)
1.9.3-p125 :006?>           end
1.9.3-p125 :007?>         self.oldeq(o)
1.9.3-p125 :008?>       end
1.9.3-p125 :009?>   end
 => nil 
1.9.3-p125 :010 > 
1.9.3-p125 :011 >   C = Struct.new(:x, :y)
 => C 
1.9.3-p125 :012 > j = C.new(1,2)
 => #<struct C x=1, y=2> 
1.9.3-p125 :013 > j == [1,2]
 => false 
1.9.3-p125 :014 > [1,2] == j
 => true 
neilconway commented 11 years ago

On reflection, I think this is fine. The issue raised in the comment is bogus (we override Struct equality so that behavior isn't accurate), and continuing to support the ability to compare Bud tuples and arrays for equality seems useful.