Best practices regarding equals: to overload or not to overload?

Question

Consider the following snippet:

import java.util.*;
public class EqualsOverload {
    public static void main(String[] args) {
        class Thing {
            final int x;
            Thing(int x)          { this.x = x; }
            public int hashCode() { return x; }

            public boolean equals(Thing other) { return this.x == other.x; }
        }
        List<Thing> myThings = Arrays.asList(new Thing(42));
        System.out.println(myThings.contains(new Thing(42))); // prints "false"
    }
}

Note that contains returns false!!! We seems to have lost our things!!

The bug, of course, is the fact that we've accidentally overloaded, instead of overridden, Object.equals(Object). If we had written class Thing as follows instead, then contains returns true as expected.

        class Thing {
            final int x;
            Thing(int x)          { this.x = x; }
            public int hashCode() { return x; }

            @Override public boolean equals(Object o) {
                return (o instanceof Thing) && (this.x == ((Thing) o).x);
            }
        }

Effective Java 2nd Edition, Item 36: Consistently use the Override annotation, uses essentially the same argument to recommend that @Override should be used consistently. This advice is good, of course, for if we had tried to declare @Override equals(Thing other) in the first snippet, our friendly little compiler would immediately point out our silly little mistake, since it's an overload, not an override.

What the book doesn't specifically cover, however, is whether overloading equals is a good idea to begin with. Essentially, there are 3 situations:

• Overload only, no override -- ALMOST CERTAINLY WRONG!
  • This is essentially the first snippet above
• Override only (no overload) -- one way to fix
  • This is essentially the second snippet above
• Overload and override combo -- another way to fix

The 3rd situation is illustrated by the following snippet:

        class Thing {
            final int x;
            Thing(int x)          { this.x = x; }
            public int hashCode() { return x; }

            public boolean equals(Thing other) { return this.x == other.x; }
            @Override public boolean equals(Object o) {
                return (o instanceof Thing) && (this.equals((Thing) o));
            }
        }

Here, even though we now have 2 equals method, there is still one equality logic, and it's located in the overload. The @Override simply delegates to the overload.

So the questions are:

• What are the pros and cons of "override only" vs "overload & override combo"?
• Is there a justification for overloading equals, or is this almost certainly a bad practice?

Answer 1

I'dont see the case for overloading equals, except that is more error-prone and harder to maintain, especially when using inheritance.

Here, it can be extremly hard to maintain reflexivity, symmetry and transitivity or to detect their inconsistencies, because you always must be aware of the actual equals method that gets invoked. Just think of a large inheritance hierarchie and only some of the types implementing their own overloading method.

So I'd say just don't do it.

Answer 2

I use this approach with override and overload combo in my projects, because code looks a bit cleaner. I didn't have problems with this approach so far.

Answer 3

If you have one single field as in your example, I think

@Override public boolean equals(Object o) {
    return (o instanceof Thing) && (this.x == ((Thing) o).x);
}

is the way to go. Anything else would be overly complicated imo. But if you add a field (and don't want to pass the 80-column recommendation by sun) it would look something like

@Override public boolean equals(Object o) {
    if (!(o instanceof Thing))
        return false;
    Thing t = (Thing) o;
    return this.x == t.x && this.y == t.y;
}

which I think is slightly uglier than

public boolean equals(Thing o) {
    return this.x == o.x && this.y == o.y;
}

@Override public boolean equals(Object o) {
    // note that you don't need this.equals().
    return (o instanceof Thing) && equals((Thing) o);
}

So my rule of thumb is basically, if need to cast it more than once in override-only, do the override-/overload-combo.

---

A secondary aspect is the runtime overhead. As Java performance programming, Part 2: The cost of casting explains:

Downcast operations (also called narrowing conversions in the Java Language Specification) convert an ancestor class reference to a subclass reference. This casting operation creates execution overhead, since Java requires that the cast be checked at runtime to make sure that it's valid.

By using the overload-/override-combo, the compiler will, in some cases (not all!) manage to do without the downcast.

---

To comment on @Snehal point, that exposing both methods possibly confuses client-side developers: Another option would be to let the overloaded equals be private. The elegance is preserved, the method can be used internally, while the interface to the client side looks as expected.

Answer 4

Issues with Overloaded Equals:

• All the Collections provided by Java ie; Set, List, Map use the overridden method for comparing two objects. So even if you overload the equals method, it doesn't solve the purpose of comparing two objects. Also, if you just overload and implement the hashcode method, it would result in erroneous behavior

• If you have both overloaded and overridden equals methods and exposing both these methods you are going to confuse the client side developers. It is by convention people believe that you are overriding the Object class

Answer 5

There are a number of items in the book that cover this. (It's not in front of me, so I'll refer to items as I remember them)

There is en example exactly using equals(..) where it is said that overloading should not be used, and if used - it should be used with care. The item about method design warns against overloading methods with the same number of arguments. So - no, don't overload equals(..)

Update: From "Effective Java" (p.44)

It is acceptable to provide such a "strongly typed" equals method in addition to the normal one as long as the two methods return the same result, but there is no compelling reason to do so.

So, it is not forbidden to do so, but it adds complexity to your class, while adding no gains.

Answer 6

I can think of a very simple example where this won't work properly and why you should never do this:

class A {
   private int x;

   public A(int x) {
       this.x = x;
   }

   public boolean equals(A other) {
       return this.x == other.x;
   }

   @Override
   public boolean equals(Object other) {
       return (other instanceof A) && equals((A) other);
   }
}

class B extends A{
    private int y;

    public B(int x, int y) {
        super(x);
        this.y = y;
    }

    public boolean equals(B other) {
        return this.equals((A)other) && this.y == other.y; 
    }

    @Override
    public boolean equals(Object other) {
        return (other instanceof B) && equals((B) other);
    }
}

public class Test {
    public static void main(String[] args) {
        A a = new B(1,1);
        B b1 = new B(1,1);
        B b2 = new B(1,2);

        // This obviously returns false
        System.out.println(b1.equals(b2));
        // What should this return? true!
        System.out.println(a.equals(b2));
        // And this? Also true!
        System.out.println(b2.equals(a));
    }
}

In this test, you can clearly see that the overloaded method does more harm than good when using inheritance. In both wrong cases the more generic equals(A a) is called, because the Java compiler only knows that a is of type A and that object does not have the overloaded equals(B b) method.

Afterthought: making the overloaded equals private does solve this problem, but was does that really gain you? It only adds an extra method, which can only be called by doing a cast.