I'm looking into bugs found by the latest version of FindBugs.
An error that has occurred in a few places is a non-symmetric equals
method. FindBugs describes the error like this:
This class defines an equals method that overrides an equals
method in a superclass. Both equals methods methods use
instanceof in the determination of whether two objects are equal.
This is fraught with peril, since it is important that the equals
method is symmetrical (in other words, a.equals(b) ==
b.equals(a)). If B is a subtype of A, and A's equals method
checks that the argument is an instanceof A, and B's equals
method checks that the argument is an instanceof B, it is quite
likely that the equivalence relation defined by these methods is
not symmetric.
There's quite a few descriptions of the problem on the web, along with some
proposed solutions:
http://www.drdobbs.com/java/184405053
http://tal.forum2.org/equals
http://www.artima.com/lejava/articles/equality.html
http://stackoverflow.com/questions/27581/overriding-equals-and-hashcode-in-java
Have others run into this same problem? How have you solved it?
I hear that the second edition of Josh's Effective Java book has a
solution. I don't have a copy, can someone who does look this up
for me?
The approach that looks the best to me so far is this described by
Gregor Zeitlinger in
http://www.artima.com/forums/flat.jsp?forum=226&thread=259279
There's a version that doesn't require canEqual and is even more flexible:
Point.equals(Object o) {
if (o instanceof Point) {
if (!o.getClass.isAssignableFrom(getClass())) {
//let the subclass decide if we are equal
return o.equals(this);
}
Point p = (Point)o;
return x == p.x && y == p.y;
}
return false;
}
now ColoredPoint could be defined in either way:
ColoredPoint.equals(Object o) {
if (o instanceof ColoredPoint) {
if (!o.getClass.isAssignableFrom(getClass())) {
//let the subclass decide if we are equal
return o.equals(this);
}
ColoredPoint p = (ColoredPoint)o;
return color == p.color && super.equals(o);
}
return false;
}
or
ColoredPoint.equals(Object o) {
if (o instanceof ColoredPoint) {
if (!o.getClass.isAssignableFrom(getClass())) {
//let the subclass decide if we are equal
return o.equals(this);
}
ColoredPoint p = (ColoredPoint)o;
return color == p.color && super.equals(o);
} else if (o instanceof Point) {
return color == Color.RED && super.equals(o);
}
return false;
}
What do others think?