hamstergem / hamster

Efficient, Immutable, Thread-Safe Collection classes for Ruby
Other
1.89k stars 95 forks source link

Equality issues #247

Open mauricioszabo opened 5 years ago

mauricioszabo commented 5 years ago

I was working on a project, then I've found a case when a == b is true but b == a is false.

Minimum code that I could find the error:

require 'hamster'
require 'bigdecimal'

a = Hamster.from({
  items: [{
    amount: 0.0,
    quantity: 1
  }]
})

b = Hamster.from({
  items: [{
    amount: BigDecimal('0.0'),
    quantity: 1
  }]
})

a == b
# => false
b == a
# => true
alexdowad commented 5 years ago

Simpler repro uses a = Hamster.from({amount: 0.0}); b = Hamster.from({amount: BigDecimal('0.0')}).

alexdowad commented 5 years ago

This is inherited from the idiosyncratic behavior of Hash#eql?. (That's ::Hash, not Hamster::Hash!)

{amount: 0.0}.eql?({amount: BigDecimal('0.0')})
# => false
{amount: BigDecimal('0.0')}.eql?({amount: 0.0})
# => true

Questions:

  1. Is that the intended behavior of #eql? for Ruby's built-in Hash?
  2. Should Hamster::Hash#== actually be using #eql?? It seems that chaining to #== would be more appropriate.
alexdowad commented 5 years ago

From ri BasicObject#==:

"Numeric types... perform type conversion across ==, but not across eql?..."

It looks like implementing Hamster::Hash#== using ::Hash#eql? is wrong. I'll just look at the git commit history to see if there was a reason why that was done.

alexdowad commented 5 years ago

Looks like I wrote that code back in 2014! Interesting to see that it has stood up for 5 years, only for a lurking bug to show up now...