Why does the equals() implementation generated by Eclipse check for null before type checking (instanceof)?

Question

I regularly used Eclipse's code generation tools (Source / Generate hashCode() and equals()...) to create the equals() implementation for simple POJO classes. If I choose to "Use instanceof to compare types" this produces an equals() implementation similar to this:

  @Override
  public boolean equals(Object obj) {
      if (this == obj) {
          return true;
      }
      if (obj == null) {
          return false;
      }
      if (!(obj instanceof MyClass)) {
          return false;
      }
      MyClass other = (MyClass) obj;
      // check the relevant fields for equality
  }

Today a colleague pointed out, that the second if statement is not necessary at all, since the instanceof type check will return false whenever obj is null. (See question 3328138.)

Now, I guess that the folks writing the code templates for Eclipse JDT are worth their salt, too. So I figure there must be some reason for that null check, but I'm not really sure what it is?

(Also question 7570764 might give a hint: if we use getClass() comparison for type checking instead instanceof, obj.getClass() is not null safe. Maybe the code template is just not clever enough to omit the null check if we use instanceof.)

EDIT: Dragan noticed in his answer, that the instanceof type check is not the default setting in Eclipse, so I edited that out of the question. But that does not change anything.

Also please do not suggest that I should use getClass() or (even better!) a different IDE. That's not the point, that does not answer the question. I didn't ask for advice on how to write an equals() implementation, whether to use instanceof or getClass(), etc.

The question roughly is: is this a minor bug in Eclipse? And if it's not, then why does it qualify as a feature?

Answer 1

It is unnecessary because instanceof has a built in null check. But instanceof is a lot more than a simple foo == null. It is a full instruction preparing a class check doing unnecessary work before the null check is done. (see for more details http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-6.html#jvms-6.5.instanceof)

So a separate null check could be a performance improvement. Did a quick measurement and no surprise foo==null is faster than a nullcheck with instanceof.

But usually you do not have a ton of nulls in an equals() leaving you with a duplicate unnecessary nullcheck most of the times... which will likely eat up any improvement made during null comparisons.

My conclusion: It is unnecessary.

Code used for testing for completeness (remember to use -Djava.compiler=NONE else you will only measure the power of java):

public class InstanceOfTest {
    public static void main(String[] args) {
        Object nullObject = null;

        long start = System.nanoTime();         
        for(int i = Integer.MAX_VALUE; i > 0; i--) {
            if (nullObject instanceof InstanceOfTest) {}
        }
        long timeused = System.nanoTime() - start;  

        long start2 = System.nanoTime();
        for(int i = Integer.MAX_VALUE; i > 0; i--) {
            if (nullObject == null) {}
        }
        long timeused2 = System.nanoTime() - start2;

        System.out.println("instanceof");
        System.out.println(timeused);       
        System.out.println("nullcheck");
        System.out.println(timeused2);
    }
}

Answer 2

Indeed, it is unnecessary and it is the mistake of the authors of the Eclipse template. And it is not the first one; I found more of smaller errors there. For example, the generation of the toString() method when I want to omit null values:

public class A {
    private Integer a;
    private Integer b;

    @Override
    public String toString() {
        StringBuilder builder = new StringBuilder();
        builder.append("A [");
        if (a != null)
            builder.append("a=").append(a).append(", ");
        if (b != null)
            builder.append("b=").append(b);
        builder.append("]");
        return builder.toString();
    }
}

If a is not null and b is, there will be an extra comma before the closing ].

So, regarding your statement: "Now, I guess that the folks writing the code templates for Eclipse JDT are worth their salt, too.", I assume they are, but it would not hurt them to pay more attention to these tiny inconsistencies. :)