My question is about the equals() method. Most people implement equals() to start like:
@Override public boolean equals(Object o) { if (this == o) { return true; } if ( (o == null) || !(o instanceof MyClass) ) { return false; } ...
The !(o instanceof MyClass) line here can be very limiting. Once a class with such an equals method is wrapped in another class, it is no-longer equal to itself! Consider the following function and wrapper class. It works like Collections.unmodifiableList(x), only I made an UnList interface that extends List to deprecate each mutator method in addition to making the method throw an UnsupportedOperationException.
/** Returns an unmodifiable version of the given list. */
public static <T> UnList<T> un(List<T> list) {
if (list == null) { return UnList.empty(); }
if (list instanceof UnList) { return (UnList<T>) list; }
if (list.size() < 1) { return UnList.empty(); }
// Declare a real class instead of an anonymous one so that we can
// check the class of the object passed to the equals() method and
// compare the inner objects if that's appropriate.
class UnListWrapper<E> implements UnList<E> {
private final List<E> inner;
private UnListWrapper(List<E> i) { inner = i; }
@Override public UnListIterator<E> listIterator(int index) {
return un(inner.listIterator(index));
}
@Override public int size() { return inner.size(); }
@Override public E get(int index) { return inner.get(index); }
@Override public int hashCode() { return inner.hashCode(); }
@Override public boolean equals(Object o) {
// Typical implementations of equals() check the exact
// class of the passed object so actually give up the
// inner, wrapped object to allow the class check to pass
// in those cases. For best results, use Sorted
// collections that don't call this method.
return inner.equals((o instanceof UnListWrapper)
? ((UnListWrapper) o).inner
: o);
}
};
return new UnListWrapper<>(list);
}
When I wrote the above equals() method, I thought that I was doing a good thing by broadening the scope where the equals method could still work. But sometimes failing fast can be kinder than working half the time.
Imagine a bunch of lists with the above implementation being thrown into a HashSet. Some are wrapped in the UnListWrapper, some are not. If you put in two equivalent unwrapped lists, or put the unwrapped list in first, then you add the wrapped one, the equals method in the wrapped list will compare its inner list and see that they are the same. But if you add the wrapped list followed by an unwrapped one, the equals method of the unwrapped List could reject the wrapped one as being the wrong class, and you'd end up with two of the same List in your set.
This order-dependent problem can be hard to test for and is an awful kind of problem to debug. So now I'm thinking that it's kinder to implement equals to fail fast:
@Override public boolean equals(Object o) {
return inner.equals(o);
}
Then if someone writes a List with an instanceof MyClass equals() implementation, they will have to fix their equals to use instanceof List which is probably better all the way around.
On the other hand, the end user might not have control over the List implementation and might not put it in a HashSet and would appreciate things working half the time.
I'm at the edge of my experience here and could use some better informed opinions. If this has no right answer, is there a best wrong answer?
P.S. I'm using the TreeSet and TreeMap implementations from Clojure so that people using this utility don't have to worry about equals() and hashCode(). But just writing JUnit tests, having a working equals() is extremely useful, 'cause Java is designed to be used that way, and a lot of Java expects it, even if using a separate Comparator is generally superior.
