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?

Dougald

Your code is a mess in the first place: why does Man decide to play with the Dog in the method feedAnimals? – Aftmost

@SimeonVisser you are right, I changed the method name. – Dougald

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.

Agneta

You apparently disagree with Josh Block on that one ... artima.com/intv/bloch17.html – Fasciculus

This article deals with another subject, strictly not linked with the open/closed principle. – Agneta

Okay, but what about Liskov? – Fasciculus

instanceof does not inhererently break either of those principles. It's just sloppy usage of it that does so (but that could be said for pretty much any language feature). – Smolensk

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);
}

Woodborer

The problem with your getPreferredFood() call is that the type system will not prevent you from doing dog.feed(cat.getPreferredFood()). This is a fundamental limitation of object-orientation. – Netti

Arguably, the point to using type systems is to prevent wrong operations (e.g., feeding onions to a cat) from even being attempted. – Netti

But lets say one Man likes to playWith Dogs, and not with Cats - would you replace the isntanceof operator with something else? is the instanceof operator wrong? – Dougald

Answer 3

instanceof is a type system escape hatch. It can be used to do really evil things, like make generics not really generic, or extend a class hierarchy with ad-hoc virtual methods that never appear in the visible interface of those classes. Both of these things are bad for long-term maintainability.

More often than not, if you find yourself wanting to use instanceof, it means that there is something wrong with your design. Breaking the type system should always be a last resort, not something to be taken lightly.

I do not think your particular example warrants using instanceof. The object-oriented way to do this is to use the visitor pattern:

abstract class Animal {
  def accept(v: AnimalVisitor)
}

trait Edible extends Animal {
  def taste : String
  def accept(v: AnimalVisitor) = v.visit(this)
}

trait Pet extends Animal {
  def growl : String
  def accept(v: AnimalVisitor) = v.visit(this)
}

abstract class AnimalVisitor {
  def visit(e: Edible)
  def visit(p: Pet)
}

class EatOrPlayVisitor {
  def visit(e: Edible) = println("it tastes " + e.taste)
  def visit(p: Pet) = println("it says: " + p.growl)
}

class Chicken extends Animal with Edible {
  def taste = "plain"
}

class Lobster extends Animal with Edible {
  def taste = "exotic"
}

class Cat extends Animal with Pet {
  def growl = "meow"
}

class Dog extends Animal with Pet {
  def growl = "woof"
}

object Main extends App {
  val v = new EatOrPlayVisitor()
  val as = List(new Chicken(), new Lobster(), new Cat(), new Dog())
  for (a <- as) a.accept(v)
}

NOTE: I am aware that Scala has case classes, but I wanted to provide a general object-oriented solution.

Netti

Thank you for the answer. But your code does a different thing then mine. The problem arises when Man gets an array of Animals, and he chooses by some logic to eat some, and to playWith some. When you Eat them all, and playWith them all, there is no problem, and also in my implementation I wouldn't have to use instanceof operator... – Dougald

@user1028741: I am going to update my answer. Please wait. :-) – Netti

Answer 4

using instance of is a bad practise because in the OOP there is no need to check what the class is, if the method is compatible you should to be able to call it with such arguments, otherwise design is spoiled, flawed, but it exist the same way as goto in C and C++, I think sometimes it might be easier to integrate a bad code using instance of but if you make your own proper code avoid it so basically this is about of programming style what is good and what is bad, when and why in some curcumstances bad style is used, because sometimes the code quality is secondary, perhaps sometimes the goal is to make the code not easy to understand by others so that would be the way to do it

Allister

The user did not imply that his subclasses had different interfaces. You just reiterate that using instanceof is bad design without describing how his task to differentiate instances by class should be implemented. There are enough valid used cases. E.g. think about creating a histogram for the incidence of subclasses. – Infective

Answer 5

After reading Radiodef’s answer, it occurred to me that the argument between instance of and a isXXX() method may be related to the Problem_of_universals, where philosophers explore the topic about "Whether universals can be considered exist?", or in the OOP context, should you emphasize the judgement itself(should you eat this animal or should you play with it) so much, that making it into an entity(like a intentionally defined method), so that you can manage it easier later.

If you threat the judgement like it doesn't exist in itself, and only rely on the subject to judge, then maintaining a bunch of instance of in each subject make the most sense.

Otherwise, emphasize the judgement as a method in an interface or even further, emphasize it as a separate interface as isekaijin's answer does.

The point is, these designs are at the cost of complexity(readability, number of classes and layers of invocation) and in exchange for easy management later.

Maybe philosophy seems a little out of topic here. IMHO, the relation between objects in OOP, especially in domain driven designs, benefits from careful modeling of real life relations and even modeling of the cognitive model of the development team members.

Ammonic