Smelly instanceof Operator

The instanceof operator in Java is used to check if a given reference is an instance of (aka object of) a given class. Though it is useful in some situations, it is a bad practice to use the instanceof operator. Whenever I see instanceof in my student's projects or a code review I raise the alarm. This article explains why the instanceof operator is considered a bad practice and how to avoid it.

Let's begin with an example of improper usage of instanceof operator. Suppose you have some Shapes and you want to print something based on the Shape type, you can use instanceof operator as shown below. Though the code smells bad, there is no compile-time error and the code will produce the expected output.
public class DrawingBoard {

    public static void main(String[] args) {
        DrawingBoard board = new DrawingBoard();
        board.draw(new Circle(7));
        board.draw(new Square(10));
    }

    public void draw(Shape shape) {
        if (shape instanceof Circle) {
            System.out.println("Circle with radius " + ((Circle)shape).getRadius());
        } else if (shape instanceof Square) {
            System.out.println("Square with width " + ((Square)shape).getWidth());
        }
    }
}

interface Shape {

}

class Circle implements Shape {
    private final int radius;

    public Circle(int radius) {
        this.radius = radius;
    }

    public int getRadius() {
        return this.radius;
    }
}

class Square implements Shape {
    private final int width;

    public Square(int width) {
        this.width = width;
    }

    public int getWidth() {
        return this.width;
    }
}
The above code smells bad because it is difficult to maintain this code. For example, if you add a new shape like a Rectangle or Triangle, you need to modify the DrawingBoard class. If you use instanceof in multiple places of your project like this, you need to modify them in all those places. I've already mentioned in my article Why should I have supertype reference & subclass object?, but repeating it here: modifying code after release is risky as you may introduce new bugs. Therefore, a software design requiring more changes to introduce a new feature is a bad design.
A good software design permits new features with less code change.
Consider the following code which uses the advantage of runtime polymorphism.
public class DrawingBoard {

    public static void main(String[] args) {
        DrawingBoard board = new DrawingBoard();
        board.draw(new Circle(7));
        board.draw(new Square(10));
    }

    public void draw(Shape shape) {
        shape.draw();
    }
}

interface Shape {
    void draw();
}

class Circle implements Shape {
    private final int radius;

    public Circle(int radius) {
        this.radius = radius;
    }

    @Override
    public void draw() {
        System.out.println("Circle with radius " + this.radius);
    }
}

class Square implements Shape {
    private final int width;

    public Square(int width) {
        this.width = width;
    }

    @Override
    public void draw() {
        System.out.println("Square with width " + this.width);
    }
}
As you can see, each shape is responsible for its own draw method and the DrawingBoard just calls the draw method. In the new design, each object knows how to draw itself and this eliminates the need of the instanceof operator. Considering the previous scenario, if you want to add a new shape: Rectangle, just go ahead and create a new class Rectangle and all your changes will be inside the Rectangle class except modifying the main method to pass a Rectangle object. If you follow Object Oriented Principles and come up with a proper design, there shouldn't be a situation to use the instanceof operator. In conclusion, using instanceof operator indicates that there is a flaw in the design.

Though it is a bad practice to use the instanceof operator, sometimes it is hard to avoid the instanceof operator. This happens frequently when you are using some third party libraries with a design flaw or some legacy code that you cannot change. For example, if the Shape and all its subclasses are defined as they are in the first example in a library that you cannot modify, you don't have an option other than using instanceof operator. People often use the instanceof operator in the equals method. Even though the Class.getName() method can be used in the equals method, using instanceof operator is acceptable as you are using it in the same class. However, if you are using the instanceof operator with your own classes to choose different implementations based on their type, then think twice and fix your design.
Latest
Previous
Next Post »

Contact Form

Name

Email *

Message *