Generating java bugs in equals methods using an IDE

Its no revelation that Eclipse, Netbeans and others can generate code. Its particularly useful if getters, setters or constructors are required. Implementing an interface is very simple indeed as these IDE will stub out the interface methods. More complex methods though require some thought. Generation of the equals method is such a case.

How to write a good equals method is well documented by Joshua Bloch in Effective Java, Chapter 3 by Joshua Bloch.

Eclipse V Netbeans who generates the best code?

Before we get deeper into the code here are two efforts at generating the equals method for this class:

public class SimpleBean  {
    private final BigDecimal bigDecimal;
    public SimpleBean(BigDecimal bigDecimal) {
        this.bigDecimal = bigDecimal;
    }
}

The results for even such a simple class are interesting.

Netbeans

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

The Netbeans attempt is reasonable at first sight. Even with the above there are a few observations and a bug which I will come to later on. Can you spot it?

• The getClass system for checking that our object can be tested for equality is a little inflexible. For classes that implement a common interface its nice to be able to compare across implementations. This is often done using the if (!(obj instanceof MyType)) check. GetClass is optimal, but it would be good to have an option in the generation.
• The style of the value check is a little compressed for some tastes.

Eclipse

  @Override
  public boolean equals(Object obj) {
      if (this == obj)
	  return true;
      if (obj == null)
	  return false;
      if (getClass() != obj.getClass())
          return false;
      SimpleBean other = (SimpleBean) obj;
      if (bigDecimal == null) {
        if (other.bigDecimal != null)
          return false;
       } else if (!bigDecimal.equals(other.bigDecimal))
          return false;
       return true;
}

Eclipse generates very similar code. The bug is still there though, can you see it yet?

• Eclipse has extra options. You can switch out that getClass for the more flexible instanceof check.
• The style above is better and can be improved with the introduction of blocks on all if statements.
• Eclipse does more optimization assuming that more often than not an object is passed in.

Neither are good! The generated equals bug exposed!

It should be clear when I write a test that will fail for either of the above implementations.

@Test
    public void testObjectInconsistantWithEquals_Equals() {
        SimpleBean testSimpleBean = new SimpleBean(new BigDecimal("0"));
        SimpleBean expectedEqualSimpleBean = new SimpleBean(new BigDecimal("0.0"));
        assertTrue("Equality Test Fail", testSimpleBean.equals(expectedEqualSimpleBean));
    }
}

The above test fails because BigDecimal has an equals implementation that will return false when it checks 100 against 100.00. Its implementation of the compare method is declared as being inconsistent with equals. This in itself is interesting but for this example it means we can modify the generated code to provide a fix using this.bigDecimal.compareTo(other.bigDecimal) != 0 instead of relying on the generated use of equals.

Recommendations

In light of the above I would suggest the following points.

• Apart from primitive and well known objects generated equals methods can be very bad indeed.
• Always write a test for equals, do not rely on an IDE.
• If you change your object, remember to re-visit the equals method.