2

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;
}
BJ Dela Cruz
  • 5,194
  • 13
  • 51
  • 84
  • 3
    Other answers have already made the right points, but just to help you when searching in the future: it's *overriding*, not *overwriting*. – Jon Skeet Feb 06 '12 at 07:04
  • possible duplicate of [Any reason to prefer getClass() over instanceof when generating .equals()?](http://stackoverflow.com/questions/596462/any-reason-to-prefer-getclass-over-instanceof-when-generating-equals) – Raedwald Apr 15 '15 at 20:06

3 Answers3

3

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.

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
1

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.

Kris
  • 5,714
  • 2
  • 27
  • 47
1

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.

Community
  • 1
  • 1
maaartinus
  • 44,714
  • 32
  • 161
  • 320