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

polygenelubricants picture polygenelubricants · May 26, 2010 · Viewed 15k times · Source

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

b_erb picture b_erb · May 26, 2010

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.