"Christoph Rippel" <crippel / primenet.com> wrote:
>
> > From: Ben Tilly [mailto:ben_tilly / hotmail.com]
[...]
> > I think that calling them unequal is quite reasonable.
>No Ben this is not reasonable (practical at least;-)
>standard (ruby gives you)
>------------------
>[["hello"], ["hello"], ["hello"]]
>[["hello"], ["hello"], ["hello"]]
>"Now equal"
>true
>------------------
>a = ["hello"]
>b = (1..3).map {|x| a}
>c = (1..3).map {|x| ["hello"]}
>
>p b,c
>p "Now equal", (b == c)
>-----------------
>Plenty of code relies on the  " b == c" semantics  -
>if you would change this all hell would break loose.
[...]

When I had time to think about it I realized that it
would indeed be bad to get answers from == that could
not be duplicated through <=>.  So I came up with the
following strategy that still handles recursion just
fine:

    class Array
      @@other_seen = nil

      def == (other)
        if @@other_seen.nil?
          # Wrap the global in a protected block
          begin
            @@other_seen = Hash.new( nil )
            return self == other
          ensure
            @@other_seen = nil
          end
        else
          return false unless other.kind_of? Array
          return false unless length == other.length
          # Detect recursion
          seen_id = @@other_seen[self.id]
          if seen_id.nil?
            seen_id = Hash.new( false )
            seen_id[other.id] = true
            @@other_seen[self.id] = seen_id
          else
            return true if seen_id[other.id]
            seen_id[other.id] = true
          end
          # Obvious test
          each_index do |i|
            return false if not self[i] == other[i]
          end
          return true
        end
      end
    end

This restores the expected behaviour.

Now the right way to do the caching strategy above
would be to make methods like <=> be able to take
1 or 2 arguments.  The optional argument would be
the recursive cache, and the above check would be
inserted into all of them.  Which would mean that
you would move the cache code into Enumerable and
then just call:

    @cache, is_seen = check_cache(@cache, self.id, other.id)

in every comparison method.

This requires a far-reaching core change.  But it
would kill the recursive algorithm bug, it would
give (IMHO of course) reasonable answers, and by
passing this as a parameters you don't need the
ugly use of class data...

Cheers,
Ben
_________________________________________________________________
Get your FREE download of MSN Explorer at http://explorer.msn.com