“instanceof”, Why And How To Avoid It In Code

Posted by Ivana Shekerova

The java “instanceof” operator is used to test whether the object is an instance of the specified type (class or subclass or interface). It is also known as type comparison operator because it compares the instance with type. It returns either true or false. If we apply this operator with any variable that has null value, it returns false.

Probably most of you have already heard that using “instanceof” is a code smell and it is considered as a bad practice. While there is nothing wrong in it and may be required at certain times, but the good design would avoid having to use this keyword.

Example of incorrect use of instanceof

 

class Animal {}
class Fish extends Animal {
  void swim(){
    System.out.println(“Swim”);
  }
}
class Bird extends Animal {
  void fly(){
    System.out.println(“Fly”);
  }
}
class Kangaroo extends Animal {
  void jump(){
    System.out.println(“Jump”);
  }
}

public final class BadUseOfInstanceOf {
  public static void main(String[] args){
     makeItMove(new Fish());
     makeItMove(new Bird());
     makeItMove(new Kangaroo());
  }

  public static void makeItMove(Animal animal){
    if (animal instanceof Fish){
      Fish fish = (Fish)animal;
      fish.swim();
    }
    else if (animal instanceof Bird){
      Bird bird = (Bird)animal;
      bird.fly();
    }
    else if (animal instanceof Kangaroo){
      Kangaroo kangaroo = (Kangaroo)animal;
      kangaroo.jump();
    }
  }
}

 

Considering this code, we can say that it is implemented in this way because we obviously want different behaviors for different types, but here although objects are treated as they are of the superclass type, that type is not a useful abstraction because it’s not enough information to determine what you want to do with the object. Also, there is a maintainability concern because if we later add a new type of animal, we may forget or not find all the places where this behavior (if/else block) may be needed depending on the type. Except this, we must admit that the readability suffers after adding numerous cases.

And then there is polymorphism which can be used to make the code better.

Example of correct use of instanceOf

class Animal {

  void move(){

    System.out.println(“Move”);

  }

}

class Fish extends Animal {

  @Override void move(){

    System.out.println(“Swim”);

  }

}

class Bird extends Animal {

  @Override void move(){

    System.out.println(“Fly”);

  }

}

class Kangaroo extends Animal {

  @Override void move(){

    System.out.println(“Jump”);

  }

}

 

public final class ProperUseOfInstanceOf {



  public static void main(String[] args){

     makeItMove(new Fish());

     makeItMove(new Bird());

     makeItMove(new Kangaroo());

  }



  /**

  * This method implementation doesn't care at all

  * about Fish/Bird/Kangaroo. It just knows that it has

  * been passed an Animal. Different versions of

  * 'move' are called, specific to each Animal.

  */

  public static void makeItMove(Animal animal){

    animal.move();

  }

}

The result may not always be the purest code, but it will be much improved, cleaner and object-oriented. The techniques can be applied more generally to avoid “instanceof” and replace if/else with polymorphism. If you want specific behavior depends on the type of object then that behavior should be encapsulated on the object itself.

Unfortunately, this isn’t always the end of the story, because this kind of code tends to show up when we are trying to implement the third-party library, and we can’t change the object in question. In this case, we will still have the instanceOf and the casting, but we can, for example, hide them to an extent with the help of the decorator pattern. Which allows dynamic wrappings of the objects in order to modify their existing responsibilities and behaviors.

Example of the use of decorator pattern

Object result = frevvoFormService.get(action);
if (result != null)
{
    if (result instanceof String)
    {
        response.setContentType("text/xml");
        response.getOutputStream().write(((String) result).getBytes(Charset.forName("UTF-8")));
        response.getOutputStream().flush();
    }
    else if (result instanceof JSONObject)
    {
        response.addHeader("X-JSON", result.toString());
        response.setContentType("application/json");
        response.getOutputStream().write(
(result.toString()).getBytes(Charset.forName("UTF-8")));
        response.getOutputStream().flush();
    }
}

 

Object result = frevvoFormService.get(action);

if (result != null)
{

       new BuildHttpResponse(result).buildResponse(response);

}



private class BuildHttpResponse {

      private Object result;

      public BuildHttpResponse(Object result){

            this.result = result;

}

public void buildResponse(HttpServletResponse response){

            if (result instanceof String)
              {
                 response.setContentType("text/xml");
                 response.getOutputStream().write(((String) result).getBytes(Charset.forName("UTF-8")));
                 response.getOutputStream().flush();
              }
              else if (result instanceof JSONObject)
              {
                 response.addHeader("X-JSON", result.toString());
                 response.setContentType("application/json");
                 response.getOutputStream().write(

(result.toString()).getBytes(Charset.forName("UTF-8")));
                 response.getOutputStream().flush();
              }

}

}

I guess that the reason why I really decided to write about this “problem” is that we can always make our code better and take small steps into improving our coding style, and end up with clean and easily maintainable code.

Comments

  • It’s true that Java intends the polymorphic way to define different actions for each objects. However, I still have that using instanceOf wins maintainability and readability.

    The fact which all the codes that need to changed are in one place in an if/else makes them readable and viewable in all without needing to jump to different places.

    And of course, for libraric classes, you cannot apply overridden codes to them without additional wrapping. This complicates the entire process.

    Using instanceOf to compare classes may have violated OOP rules, but it does not win over readability and maintainability in this case.

    • “Using instanceOf to compare classes may have violated OOP rules, but it does not win over readability and maintainability in this case.” Well, actually, it does.

      Consider following example: we have some sort of chat. We do not know who users are, but we need to send them our message. String or Json or custom class, doesn’t matter. In the way of instanceOf, we create a bunch of classes representing Socket users, WebSocketUsers, pure http user or whatever and when we need to send them our message, we check what user it is in if-else block of code and act accordingly. Then we add logic for receiving their messages. We copypaste gigantic block of if-else and fill in. However then we add another type of user and another one (e.g. we want to add async users, who dont always have stable connection/talk one time in several days). We add another check and now our block of code is well over hundreds of lines.

      Maintainability problems: forget to add somewhere (which was in post, so wont focus on that), endless conflicts when merging while working orienting on different users. I am working only on tcp users, they have some weird bug. I need to find among all of those checks block of code representing socket user, and heaven forbid I lose my focus, I will have to look back where I started.

      Readability: why do I need to read dozens of implementations of some things when I just need to read about several?

      I know, points are not structured and I am not a good explainer, however it all could be improved with the healthy dose of abstraction. Just introduce interface “CommunicationParticipant” or whatever and give it #send and #receive methods. Thats it, you now have separated logic for your users, one team that works on async can work in their part of the project and you in another. Much simpler and much sweeter. I know that I mostly reiterated some points, but hope it explained my point of view.

  • There is no “instanceOf” operator
    only lowercase “instanceof” operator.
    it’s confusing in the Java context.

    • The title has been corrected with instanceof. In the code sample, it was instanceof so hopefully, this did not cause to much frustration.

  • Java is case sensitive, it is not instanceOf is instanceof

    • Understood. The title has been corrected.

Leave a Reply