Why not use instanceof operator in OOP design?

Question

It has been repeatedly said that the instanceof operator should not be used except in the equals() method, otherwise it's a bad OOP design.

Some wrote that this is a heavy operation, but it seems that, at least java, handles it pretty well (even more efficiently than Object.toString() comparison).

Can someone please explain, or direct me to some article which explains why is it a bad design?

Consider this:

Class Man{
  doThingsWithAnimals(List<Animal> animals){
    for(Animal animal : animals){
      if(animal instanceOf Fish){
        eatIt(animal);
      }
      else if(animal instanceof Dog){
        playWithIt(animal);
      }
    }
  }
  ...
}

The decision of what to do with the Animal, is up to the Man. Man's desires can also change occasionally, deciding to eat the Dog, and play with the Fish, while the Animals don't change.

If you think the instanceof operator is not the correct OOP design here, please tell how would you do it without the instanceof, and why?

Answer 1

instanceof simply breaks the Open/Close principle. and/or Liskov substitution principle

If we are not enough abstract because of instanceof usage, each time a new subclass makes an entrance, the main code gathering the logic of the application might be updated. This is clearly not what we want, since it could potentially break the existing code and reduce its reusability.

Therefore, a good usage of polymorphism should be preferred over the basic use of conditional.

Answer 2

There's a good blog post called When Polymorphism Fails which is about this kind of scenario. Basically, you're right that it should be up to the Man to decide what to do with each kind of Animal. Otherwise, the code becomes fragmented and you end up violating principles such as Single Responsibility and Law of Demeter.

It wouldn't make sense to have code such as e.g. the following:

abstract class Animal {
    abstract void interactWith(Man man);
}
class Fish extends Animal {
    @Override
    void interactWith(Man man) {
        man.eat(this);
    }
}
class Dog extends Animal {
    @Override
    void interactWith(Man man) {
        man.playWith(this);
    }
}

In that example, we're putting Man's logic outside of the Man class.

The problem with instanceof is that if you have a large amount of Animals, you'll end up with a long if-else-if for every one of them. It's hard to maintain and prone to errors where e.g. a new type of Animal is added, but you forget to add it to the if-else-if chain. (The visitor pattern is partly a solution to the latter problem, because when you add a new type to the visitor class, all of the implementations stop compiling and you're forced to go update them all.)

However, we can still use polymorphism to make the code simpler and avoid instanceof.

For example, if we had a feeding routine such as:

if (animal instanceof Cat) {
    animal.eat(catFood);
} else if (animal instanceof Dog) {
    animal.eat(dogFood);
} else if (...) {
    ...
}

We could eliminate the if-else-if by having methods such as Animal.eat(Food) and Animal.getPreferredFood():

animal.eat(animal.getPreferredFood());

With methods such as Animal.isFood() and Animal.isPet(), the example in the question could be written without instanceof as:

if (animal.isFood()) {
    eatIt(animal);
} else if (animal.isPet()) {
    playWithIt(animal);
}