Search This Blog

Friday 10 May 2013

instanceof operator and equals method

I always based my equals implementation on the code written in the String class.
public boolean equals(final Object anObject) {

      if (this == anObject) {
         return true;
      }

      if (anObject instanceof String) {
         if (<equal by logic>) {
            return true;
         } else {
            return false;
         }
      }
      return false;
   }
The method was nicely crafted covering the expected cases well:
  1. The first if check ensured that any null input would fail.
  2. If the call was reflexive (a.equals(a)) then the method would return true immediately. This avoids the execution of business logic, since the method will always return true. 
  3. The cast check also ensures that only String instances enter the actual logic block where the comparisons are made.
The only thing I didn't like is the multiple return statements in code. So with some modifications, my equals methods all have a very standard template:
public boolean equals(final Object anObject) {

      boolean equal = false;
      if (this == anObject) {
         equal = true;
      } else if (anObject instanceof <class>) {
         if (<equal by logic>) {
            equal = true;
         }
      }
      return equal;
   }
Everything seemed good. I used this template for several Hibernate entities, DTO classes and POJOs. Things worked fine.
Until ...... until my code got reviewed.
The reviewer pointed out a bug - my use of instanceof operator made the equals method unsafe. I thought about the feedback and realized that the guy was right.
Consider the below class:
class Human {

   protected final String name;

   public Human(final String name) {
      this.name = name;
   }

   public int hashCode() {
      return this.name.hashCode();
   }

   public boolean equals(final Object anObject) {
      boolean equal = false;
      if (this == anObject) {
         equal = true;
      } else if (anObject instanceof Human) {
         final Human otherHuman = (Human) anObject;
         if (this.name.equals(otherHuman.name)) {
            equal = true;
         }
      }
      return equal;
   }
}
Here the name property of Human instance is used to decide if the two objects are equal. I decided to extend this class to build Employee.
class Employee extends Human {

   protected final String title;

   public Employee(final String name, final String title) {
      super(name);
      this.title = title;
   }

   public boolean equals(final Object anObject) {
      boolean equal = false;

      if (this == anObject) {
         equal = true;
      } else if (anObject instanceof Employee) {
         equal = super.equals(anObject);
      }
      return equal;
   }

}
I decided to test the two instances of the class for equality:
public static void main(final String[] args) {
      final Human human = new Human("Rajesh");
      final Employee employee = new Employee("Rajesh", "Engineer");

      System.out.println("human.equals(employee) ? " + human.equals(employee));
      System.out.println("employee.equals(human) ? " + employee.equals(human));
   }
The output is as below:
human.equals(employee) ? true
employee.equals(human) ? false
The two objects are not equal in the symmetric sense. This breaks the equals contract.
The problem occurred because in the call to equals method of Human class, the Employee instance passed the instanceof check. This happened because subclasses in java return true for the instanceof check of their parent classes. So while human object resolved the call to return true, the employee object's equal method failed at the instanceof check returning false.
To fix the above problem I made some changes to the equals method of Human class:
public boolean equals(final Object anObject) {
      boolean equal = false;
      if (this == anObject) {
         equal = true;
      } else if ((null != anObject) && (anObject.getClass() == Human.class)) {
         final Human otherHuman = (Human) anObject;
         if (this.name.equals(otherHuman.name)) {
            equal = true;
         }
      }
      return equal;
   }
This will now ensure that no subclass passes the equals check.
Question: So why do the Sun developers use the instanceof check in String ?
Answer: Those guys are smart. The String class is marked final. So no one can extend it and hence the instanceof check will never fail because of the subclass condition.
So if we are working with final classes, than instanceof check is fine, else the getClass method is a better option.

No comments:

Post a Comment