As I've worked with legacy Java code over the years, I've run into subtle logic and performance issues that could be traced back to improperly overridden Object.equals(Object) methods. Although the concept behind the "equals" method is seemingly simple, Josh Bloch points out in Effective Java that "Overriding the equals method seems simple, but there are many ways to get it wrong, and the consequences can be dire. The easiest way to avoid problems is not to override the equals method, in which case each instance is equal only to itself." In this post, I look at one of "the many ways" to get an equals(Object) wrong: failing to compare exactly the same characteristics of the two objects being evaluated for equality.

The next code listing is for class MismatchedFieldAccessor. This class's equals(Object) method is flawed because it compares the class's direct attribute someString to the value retrieved from the other object's getSomeString(). In most Java classes, comparing a class's field to its accessor/get method will work properly because the accessor/get method simply returns the associated field. In this example class, however, the accessor/get method does more than simply returning the field and that makes the comparison of the field to the get/accessor method in the equals(Object) method inconsistent. (Note that the idea of having a "get" method do this type of thing is not being recommended here, but merely exists as a simple-to-understand example.)

package dustin.examples.brokenequals;

import java.util.Objects;

/**
 * Demonstrate problem with mismatched field/accessor in
 * overridden equals(Object) implementation.
 */
public final class MismatchedFieldAccessor
{
   private final String someString;

   public MismatchedFieldAccessor(final String newString)
   {
      someString = newString;
   }

   public String getSomeString()
   {
      return someString != null ? someString : "";
   }

   @Override
   public boolean equals(final Object other)
   {
      if (this == other)
      {
         return true;
      }
      if (other == null || getClass() != other.getClass())
      {
         return false;
      }

      final MismatchedFieldAccessor that = (MismatchedFieldAccessor) other;

      return Objects.equals(this.someString, that.getSomeString());
   }

   @Override
   public int hashCode()
   {
      return someString != null ? someString.hashCode() : 0;
   }
}

The above class will fail if tested with an appropriate unit test. The two unit tests listed in the next code listing point out issues with the class's equals method.

public void testEqualsOnConstructedWithNull()
{
   final MismatchedFieldAccessor accessor = new MismatchedFieldAccessor(null);
   Assert.assertEquals(null, accessor.getSomeString());
}

@Test
public void testEqualsWithEqualsVerifier()
{
   EqualsVerifier.forClass(MismatchedFieldAccessor.class).verify();
}

The first unit test, shown above, fails with this message:

java.lang.AssertionError: 
Expected :null
Actual   :

The second unit test makes use of the handy EqualsVerifier library to identify an issue with this equals(Object) implementation (emphasis added):

java.lang.AssertionError: Reflexivity: object does not equal an identical copy of itself:
  dustin.examples.brokenequals.MismatchedFieldAccessor@0
If this is intentional, consider suppressing Warning.IDENTICAL_COPY
For more information, go to: http://www.jqno.nl/equalsverifier/errormessages

 at nl.jqno.equalsverifier.EqualsVerifier.handleError(EqualsVerifier.java:381)
 at nl.jqno.equalsverifier.EqualsVerifier.verify(EqualsVerifier.java:367)
 at dustin.examples.brokenequals.MismatchedFieldAccessorTest.testEqualsWithEqualsVerifier(MismatchedFieldAccessorTest.java:36)

This post has covered one of the many ways in which an equals method can go bad if not carefully implemented, reviewed, and tested. Fortunately, the fix for this particular problem is easy: always compare the exact same field or same method's returned object of the two instances being compared for equality. In the example used in this post, comparing the two "someString" fields directly would have made the "equals" method work properly.