Geertjan's Blog

  • July 17, 2010

How Evil is "instanceof"?

Geertjan Wielenga
Product Manager
My understanding is that "instanceof" should be avoided, in favor of using the NetBeans Lookup. However, what about this situation—we have a node for libraries, with different subnodes, depending on the instance of the object in question, which is either Book or Borrower:

Here's the code I used for the above scenario:

public class LibraryNode extends AbstractNode {
public LibraryNode(Library library) {
super(Children.create(new SubChildFactory(library), true));
private static class SubChildFactory extends ChildFactory<Object> {
private final Library library;
private SubChildFactory(Library library) {
this.library = library;
protected boolean createKeys(List list) {
EList<Book> books = library.getBooks();
for (Book book : books) {
EList<Borrower> borrowers = library.getBorrowers();
for (Borrower borrower : borrowers) {
return true;
protected Node createNodeForKey(Object key) {
BeanNode childNode = null;
try {
childNode = new BeanNode(key);
if (key instanceof Book) {
Book book = (Book) key;
} else if (key instanceof Borrower) {
Borrower borrower = (Borrower) key;
} catch (IntrospectionException ex) {
return childNode;

Is the above evil and how should it be rewritten to use Lookup? I guess FilterNode should be used.

Update. See part 2 for a solution to the above problem!

Join the discussion

Comments ( 12 )
  • Integrating Stuff Saturday, July 17, 2010

    You could make a Labeled interface, that both Book and Borrower would need to implement. Together with some other changes, a lookup map for the pics for example with the key being the Class, you could avoid all instanceof/casts.

    However, one of the biggest problems with using instanceof is that it does not match on proxy objects and remote objects and the like. For example, when using the popular persistance tool Hibernate, if you fetch a Book object from the database, Hibernate might fetch this object lazily, and make a proxy for it instead of the real object. This object will not match the "instanceof" operator. It will have the same properties and it will implement the same interfaces though.

    The solution to this is to always implement to an interface if you are going to use instanceof. Make a BookInterface interface for example(or even better, make Book the interface and BookImpl the implementation class). Make Book implement this interface and change your instanceof code to "instanceof BookInterface". Your code will always work as expected, making instanceof suddenly a lot less evil.

  • Sam Griffith Saturday, July 17, 2010

    Actually it's not that it is Evil. But there is a way that this can be avoided. YOu can have subclasses of BeanNode for each type of class your going to display that give back the correct values for 'displayName' and 'iconBase'. That way they all support the BeanNode Interface, but you don't need to do instanceOf anymore.

  • Sam Griffith Saturday, July 17, 2010

    I wanted to clarify and add to my last answer.

    Your use of 'instanceOf' is not Evil per say, but using some subclasses and a Factory pattern, I think you can avoid having to check the classes using instanceOf. You'll end up with a system that can handle new node types without you having to change this code to deal with it anymore either.

    The Factory can use reflection to get the correct subclass of BeanNode to create and then your setters and getters are customized to do the right thing for that type of domain object for the BeanNode subclass.

    Sorry I was so unclear before...

  • Tim Boudreau Sunday, July 18, 2010

    I'd say it's just practical in this scenario - if you'd have to jump through more hoops or write more code to avoid an instanceof check, it's probably not worth it.

    Where instanceof gets evil is when you start having lots of methods that take parameters of type Object - and this is because there is nothing constraining what might be passed in, and anybody who has to maintain the code will have to go search for all the things that actually \*are\* passed to try to figure out what the code should do - and all the ones you can find doesn't mean all the ones there are.


  • William Woody Sunday, July 18, 2010

    I concur with the other comments.

    The 'instanceof' operator is not evil per-se. But any time I see the pattern:

    Object obj = ...;

    if (obj instanceof ClassA) { ...

    } else if (obj instanceof ClassB) { ...

    } else if (obj instanceof ClassC) { ...


    Generally I'd say ClassA, ClassB and ClassC should be refactored to inherit from a common abstract class or interface, and the code rewritten inside the {...} in terms of the methods in that interface. Thus:

    CommonInterface obj = ...;

    obj.doThing(); // or whatever is going on here.

    The beauty of this is that then it's easy to add a new ClassD.

    So in your code becomes:

    protected Node createNodeForKey(Displayable key) {

    BeanNode childNode = null;

    try {

    childNode = new BeanNode(key);



    } catch (IntrospectionException ex) {



    return childNode;


    With the appropriate changes to the Book and Borrower classes to implement the specified interface.

  • Jeffrey Smith Sunday, July 18, 2010


    I try to design my polymorphic types as I described in the above apidesign article. However, it requires that the software engineer have total control over the type design. It won't work with "foreign" types that cannot be retrofitted for the double dispatch design pattern.

    I definitely try to avoid "instanceof" as much as possible, because it is a red flag for "procedural" versus "object-oriented" code. Sometimes the "cure" is worse than the "disease" of "instanceof".

  • Aljoscha Rittner Sunday, July 18, 2010

    The question is: Is it the right hierarchy?

    Add under your library two additional nodes: Books and Borrowers. And any question about wrong use of instanceof is gone ;-)

    Sometimes it's impossibe to avoid instanceof. And it's also not a good idea to replace it with a null-test on a lookup-result. But the most instanceof tests in business logics are structural problems. In core system development it is useful to test with instanceof in convenience methods.

    br, josh.

  • Geertjan Wielenga Monday, July 19, 2010

    Thanks all for the thoughts and contributions! Here is a solution I like, by Aljoscha Rittner:


  • Pierre Thibault Monday, July 19, 2010

    You could create a Renderer:

    interface Renderer


    void setDisplayName(String name);

    void setIcon(Resource location); }

    with a renderer service:

    interface RendererService


    void registerRenderer(Renderer renderer,

    Class<T extends AbstractNode> clazz);

    void renderer(AbstractNode node);


    You would register all the renderers in the rendererService. The service would have a set of renderers and would choose the proper renderer using the class of the AbstractNode.

  • Aljoscha Rittner Monday, July 19, 2010

    Hi Pierre Thibault!

    Nodes are presentation layers for objects. A node is not a renderer for data. A node gives only a hint for a human readable presentation. To define a better representation for e.g. a Book you can use a FilterNode.

    But it's possible to add renderers for properties.

    br, josh.

  • William Woody Monday, July 19, 2010


    The only problem I see with the RendererService and Renderer class as you suggested would be the implementation of the registerRenderer.

    "The service would have a set of renderers and would choose the proper renderer using the class of the AbstractNode."

    This sounds like you're simply wrapping the ugly "if (node instanceof ClassA)... if (node instanceof ClassB)" in another class.

    My thinking has always been (a) eliminate things that create problems with reuse by properly segregating concerns, and (b) use as few classes as possible while maintaining (a).

    And while your Renderer/RendererService is interesting, in this case it seems to violate (a), since the problem with "if (node instanceof ClassA)..." is that it makes reuse difficult, and it violates (b) by rewriting the original function in terms of two separate classes.

  • Juggler Thursday, July 22, 2010

    I would use a visitor pattern or some similar

Please enter your name.Please provide a valid email address.Please enter a comment.CAPTCHA challenge response provided was incorrect. Please try again.