BJ Dela Cruz
BJ Dela Cruz

Reputation: 5354

Overriding equals() in subclass of class that contains an overridden equals()

I have a Point class and a MinesweeperSquare class, the latter being a subclass of the former. If I override the equals method in the latter, like so:

if (!(obj instanceof MinesweeperSquare)) {
  return false;
}

FindBugs reports that:

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.

In the Point class, I wrote:

if (!(obj instanceof Point)) {
  return false;
}

How do I write an equals method in MinesweeperSquare such that the equals method is symmetric?

Update

FindBugs reports no errors if I write the following in MinesweeperSquare:

if (o == null || this.getClass() != o.getClass()) {
  return false;
}

Upvotes: 2

Views: 1914

Answers (3)

maaartinus
maaartinus

Reputation: 46482

Using instanceof is fine if done correctly, which is quite hard without proper lecture. Using

this.getClass().equals(that.getClass())

is trivial, but not always does what you need. Look here for canEqual.

EDIT

This all applies only when you control both classes, which doesn't seem to be the case here. So stick with the easy way.

Upvotes: 1

Louis Wasserman
Louis Wasserman

Reputation: 198211

In your new method, MinesweeperSquare.equals(Point) will always return false, but Point.equals(MinesweeperSquare) could return true. Good on you for using FindBugs on this sort of thing. You might be able to use getClass() in both the definition of Point and MinesweeperSquare to check if the classes are exactly equal...although this, too, is tricky.

Upvotes: 3

Kris
Kris

Reputation: 5792

As you have already cited, if you use 'instanceof' operator in your equals() method implementation it become non-symetric. Get rid of 'instanceof' in your superclass as well as all subclasses and try to override equals() method based on the class properties and common sense. Most IDE's allows you to generate equals() method automatically which is a good starting point.

Upvotes: 1

Related Questions