My hovercraft is full of eels: How To Write eql?() in Ruby

So, as I've been delving into Ruby more and more of late and I needed to write an eql?() method for one of my classes. (eql?() is the Ruby equivalent of equals() in Java.)

Understanding how to write an equals() method in Java is one of the most fundamental yet poorly understood practices (if you're not worried about equals() or hashCode() then I suggest you never use a HashMap) and I wondered if the same was true of Ruby.

The most common mistake looks like this:

public boolean equals(Object object) {
    if (this == object) {
        return true;
    } else if (!(object instanceof MyClass)) {
        return false;
    }

    MyClass other = (MyClass) object;

    return this.x == other.x && this.y == other.y;
}

Spot the mistake? It's subtle yet very VERY wrong. Thankfully—now that we've largely managed to get over our obsession with deep inheritence hierarchies—it's effects aren't felt that often. The problem is that equals() needs to be symmetric: a.equals(b) should return the same result as b.equals(a).

Now, imagine a class Person with attributes of name and address and another class Employee extends Person with an extra attribute of company. Given the previous implementation of equals, what do you think the chances that the following two statements will return the same result?

person.equals(employee);
employee.equals(person);

So anyway, unlikely or not, it's still no excuse not to be precise so, in Java the canonical form of an equals() method should look roughly—give or take some formatting and style—like:

public boolean equals(Object object) {
    if (this == object) {
        return true;
    } else if (object == null || getClass() != object.getClass()) {
        return false;
    }

    MyClass other = (MyClass) object;

    return this.x == other.x && this.y == other.y;
}

Now we're comparing based on the actual class and as such the implementation is symmetric; no more problems.

Assuming I still know next to nothing about Ruby, I decided I wouldn't let my Java habits get in the way of learning something new so I searched the 'net for some examples of how to implement eql>().

After failing dismally to find anything non-trivial, I realised that Rails probably had something in ActiveRecord::Base and I was rewarded with:

def eql?(object)
  self == (object)
end

def ==(object)
  object.equal?(self) ||
    (object.instance_of?(self.class) &&
      object.id == id &&
      !object.new_record?)
end

Nothing too outrageous: eql?() simply delegates to == (nice syntactic sugar); and == does pretty much the same thing as we would have done in Java only a little more terse. (object.instance_of?(self.class) is Ruby's way of saying getClass() == object.getClass(); the Ruby equivalent of Java's instanceof is kind_of?().) We could easily re-write this—although I wouldn't in practice—as:

def ==(object)
  if object.equal?(self)
    return true
  elsif !object.instance_of?(self.class)
    return false
  end
  
  return object.id == id && !object.new_record?
end

So it seems—I trust the Rails guys to get this stuff right— that the same rules apply in Ruby as in Java. (I had no real reason to suspect otherwise but hey, it s nice to have it confirmed.)

Now if only someone could explain to me why Hash#reject() and Hash#select() aren't symmetric: one returns a Hash; the other an Array.

Comments

Actually, you can read the mistake another way:

1) You don't need the null check with an "instance of" check - null is never an instance of anything (including Object)

2) It's fine to use "instance of" if the equals() method is final (and doesn't use any obviously silly methods); if the parent class knows all the interesting state information, then that's fine. Simply put: don't put a non-final equals method on a non-final class.

Using the class equality technique has its own problems, particualarly if you use Hibernate (or other cglib-based tool). Proxy classes are subclasses, but you tend to want them to be considered "equal" to the non-proxied version.

I find it interesting that the Rails folk use the identity column for equality - the Hibernate folks suggest not doing this (because you can't check to see if unsaved objects are equal)

Posted by: Robert Watkins | December 29, 2005 01:12 AM

Hey Robert.

1) Agreed. An oversight on my part as I copied the "good" version and modified it. That said, it was supposed to be the "bad" way ;-)

2) While technically correct, that's a bit like allowing a child to play with a fully loaded sem-automatic pistol because "the safety is on". Ok, so I'm possibly being a bit melodramatic. Regarding final equals, I'd suggest NEVER making equals final; rather just make the class final instead. In either case I still recommend this form of equals() unless you have a VERY good reason not to do so (potential performance problems are, IMHO, NOT a good reason: prove it). If developers (especially junior ones) get into the habit of doing things correctly and being precise and not optimising for no good reason they are less likely to hurt themselves and those around them :).

3) As much as I dislike EJBs, that's precicely why they had the isEqual() method: sometimes the abstraction ("transparent" persistence in this case) starts to leak. You'll also notice that the Rails code has a call to new_record?() in case it has yet to be saved.

Cheers,

Simon

Posted by: Simon Harris | December 29, 2005 07:45 AM

Simon, re: 2) It depends entirely on your context.

In Hibernate, if you want to use lazy self-proxies, you need to have non-final classes because the real classes are dynamically created subclasses. In this mode, you can't make the class final, and you can't use class-level equality. Instanceof is the only way to go in this situation.

Posted by: Robert Watkins | January 3, 2006 04:50 PM

Hey Simon -

I understand what you're trying to protect against with the getClass() comparison, but I think you're missing the mark a bit. The problem that you're trying to solve is ultimately caused by a mis-use of inheritance: if there is truly an "is-a" relationship then a user should be able to freely (and confidentally) compare two instances which would pass an instanceof test while failing the getClass() comparison.

Now, if you end up with something that isn't 100% is-a, then perhaps something like composition would be more appropriate. In your example, Employee > Person has gone beyond an "is-a" relationship (and thus has extended beyond the intended use of the classic example) and is now more of a "has-a". Inheritance goes from useful to harmful in this case...

...but mainly for the implementor/user of Employee. If you want to protect the innocent, then maybe you should mark your person class final (as you suggest) or mark equals final (but be sure it's not using any overridable methods).

I'd hesitate to take the ActiveRecord implementation as gospel - I feel that they've got some ulterior motives in play. Since we're talking about a framework that maps classes into database tables, it would seem that it's very important to them that a Person is a Person and an Employee is an Employee. I'd love to see David HH provide a comment about this!

Posted by: Ryan Dlugosz | January 4, 2006 07:16 AM

Ryan, I agree, the specific example was lame. I wouldn't have coded it that way either but on the spur of the moment it was the first thing I thought of. :)

The real point I was trying to make however, was two-fold: 1) Ruby and Java aren't that dissimilar; and 2) equals() MUST be symmetric - if your implementation guarantees that then great; if not, it's broken :)

Posted by: Simon Harris | January 6, 2006 01:20 AM

I'm growing less fond of .equals() as time goes on - in java (and presumably most languages), there is the notion of a Comparator, but where is the Equator interface? Why can't I create a standard collection with a specific way of testing equality?

Somtimes I want to know if a object is "the same data" - other times I want to know if it's exactly the same db record (using id equality) - or even the exact same object (for caches etc).

And I don't want to have to create an arbitrary ordering and stuff it into a comparator either. Besides, this doesn't work. The first Equator to write is the equals equator (delegates to the objects). Then there's the TransformingEquator that uses a transformer to transform objects into different types, and then equates them (useful for uncommon/most equating situations).

I am going to see how it goes in my new projects - I will be attempting to shun .equals() as I attempt to shun .toString() (except for debugging purposes). The downside is that all the collections have to be rewritten to delegate to the equators, or wrap objects... or something.

I just think it'd be nice and declarative (and far more useful).

Posted by: Stephen Kestle | January 24, 2006 11:05 AM

My hovercraft is full of eels

The occasional public airing of my not so occasional rants

How To Write eql?() in Ruby

TrackBack

Comments

Post a comment