Is instanceof considered bad practice? If so, under what circumstances is instanceof still preferable?

Solution 1:

It's definitely has its place in a stock implementation of equals. E.g.

public boolean equals ( Object o )
{
  if ( this == o )
  {
     return true;
  }

  if ( ! (o instanceof MyClass) )
  {
    return false;
  }

  // Compare fields
  ...
}

One neat thing to know about instanceof is that its LHS can be null and in that case the expression evaluates to false.

Solution 2:

I can imagine some cases, for example you have some objects of a library, which you can't extend (or it would be inconvenient to do so), perhaps mixed with some objects of your, all with same base class, together in a collection.
I suppose that in such case, using instanceof to distinguish some processing on these objects might be useful.

Idem in some maintenance of legacy code where you cannot inject some new behavior in lot of old classes just to add a new little feature or some bug fix...

Solution 3:

I think that when you absolutely need to know the type of an object, instanceof is the best option available.

A bad practice would be to have a lot of instanceofs, one next to the other, and according to them call different methods of the objects (of course casting). This would probably reflect that the hierarchy needs rethinking and probably refactoring.

Solution 4:

When you are inside a pure OO model, then instanceof is definitely a code smell.

If, however, you are not using a 100% OO model or you need to inject stuff into it from the outside, then instanceof or equivalents (isXXX(), getType(), ...) can have its uses.

The general "rule" would be to avoid it whenever possible, especially when you control the type hierarchy and can use subtype polymorphism for example. The idea is not to ask the object what type it is and do something with it, but rather to ask the object directly or indirectly via a Visitor (essentially double polymorphism) to perform some action.

Solution 5:

It can well be used as a sanity check before casting; in addition to checking that your object is of right type, it also does check that it's not null.

if (o instanceof MyThing) {
    ((MyThing) o).doSomething(); // This is now guaranteed to work.
} else {
    // Do something else, but don't crash onto ClassCast- or NullPointerException.
}