Avoiding instanceof in Java

Question

Having a chain of "instanceof" operations is considered a "code smell". The standard answer is "use polymorphism". How would I do it in this case?

There are a number of subclasses of a base class; none of them are under my control. An analogous situation would be with the Java classes Integer, Double, BigDecimal etc.

if (obj instanceof Integer) {NumberStuff.handle((Integer)obj);}
else if (obj instanceof BigDecimal) {BigDecimalStuff.handle((BigDecimal)obj);}
else if (obj instanceof Double) {DoubleStuff.handle((Double)obj);}

I do have control over NumberStuff and so on.

I don't want to use many lines of code where a few lines would do. (Sometimes I make a HashMap mapping Integer.class to an instance of IntegerStuff, BigDecimal.class to an instance of BigDecimalStuff etc. But today I want something simpler.)

I'd like something as simple as this:

public static handle(Integer num) { ... }
public static handle(BigDecimal num) { ... }

But Java just doesn't work that way.

I'd like to use static methods when formatting. The things I'm formatting are composite, where a Thing1 can contain an array Thing2s and a Thing2 can contain an array of Thing1s. I had a problem when I implemented my formatters like this:

class Thing1Formatter {
  private static Thing2Formatter thing2Formatter = new Thing2Formatter();
  public format(Thing thing) {
      thing2Formatter.format(thing.innerThing2);
  }
}
class Thing2Formatter {
  private static Thing1Formatter thing1Formatter = new Thing1Formatter();
  public format(Thing2 thing) {
      thing1Formatter.format(thing.innerThing1);
  }
}

Yes, I know the HashMap and a bit more code can fix that too. But the "instanceof" seems so readable and maintainable by comparison. Is there anything simple but not smelly?

Note added 5/10/2010:

It turns out that new subclasses will probably be added in the future, and my existing code will have to handle them gracefully. The HashMap on Class won't work in that case because the Class won't be found. A chain of if statements, starting with the most specific and ending with the most general, is probably the best after all:

if (obj instanceof SubClass1) {
    // Handle all the methods and properties of SubClass1
} else if (obj instanceof SubClass2) {
    // Handle all the methods and properties of SubClass2
} else if (obj instanceof Interface3) {
    // Unknown class but it implements Interface3
    // so handle those methods and properties
} else if (obj instanceof Interface4) {
    // likewise.  May want to also handle case of
    // object that implements both interfaces.
} else {
    // New (unknown) subclass; do what I can with the base class
}

Selfdiscipline

I'd suggest a [visitor pattern][1]. [1]: en.wikipedia.org/wiki/Visitor_pattern – Idyllic

Is it the ifthen chain you object to, or simply the use of "instanceof"? – Mccomas

The Visitor pattern requires adding a method to the target class (Integer for instance) -- easy in JavaScript, hard in Java. Excellent pattern when designing the target classes; not so easy when trying to teach an old Class new tricks. – Selfdiscipline

@lexicore: markdown in comments is limited. Use [text](link) to post links in comments. – Falconer

"But Java just doesn't work that way." Maybe I'm misunderstanding things, but Java supports method overloading (even on static methods) just fine... it's just that your methods above are missing the return type. – Soloman

Here's what Java doesn't allow: public static void handle(Integer i) {...} public static void handle (Double d) { ... } public static void main (String[] args) { Number n = new Integer(1); handle(n); }. – Selfdiscipline

@MarkLutton Then create a wrapper and add it to that. – Shorthand

@Soloman Overload resolution is static at compile-time. – Hydrangea

Any solution in 2022?? – Adventurer

Answer 1

You might be interested in this entry from Steve Yegge's Amazon blog: "when polymorphism fails". Essentially he's addressing cases like this, when polymorphism causes more trouble than it solves.

The issue is that to use polymorphism you have to make the logic of "handle" part of each 'switching' class - i.e. Integer etc. in this case. Clearly this is not practical. Sometimes it isn't even logically the right place to put the code. He recommends the 'instanceof' approach as being the lesser of several evils.

As with all cases where you are forced to write smelly code, keep it buttoned up in one method (or at most one class) so that the smell doesn't leak out.

Peewit

Polymorphism does not fail. Rather, Steve Yegge fails to invent the Visitor pattern, which is the perfect replacement for instanceof. – Walloping

I don't see how visitor helps here. The point is that the response of OpinionatedElf to NewMonster shouldn't be encoded in NewMonster, but in OpinionatedElf. – Peewit

If Monster is made visitable, the elf can visit it to extract all the information it needs, thus keeping the logic in the OpinionatedElf class without resorting to the dangerous practices of using reflection. – Walloping

You miss the point. If OpinionatedElf could use the public interface of each Monster to make it's decision there wouldn't be an issue in the first place. – Peewit

And why OpinionatedElf can't use the public interface of each Monster? It has to depend on it to do instanceof anyway. I totally miss it now. – Walloping

The point of the example is that OpinionatedElf cannot tell from available data whether it likes or dislikes the Monster. It has to know what Class the Monster belongs to. That requires either an instanceof, or the Monster has to know in some way whether the OpinionatedElf likes it. Visitor doesn't get round that. – Peewit

@Peewit Visitor pattern does get around that by adding a method to Monster class, responsibility of which is basically to introduce the object, like "Hello, I am an Orc. What do you think about me?". Opinionated elf can then judge monsters based on these "greetings", with a code similar to bool visitOrc(Orc orc) { return orc.stench()<threshold; } bool visitFlower(Flower flower) { return flower.colour==magenta; }. The only monster-specific code will then be class Orc { <T> T accept(MonsterVisitor<T> v) { v.visitOrc(this); } }, enough for every monster inspection once and for all. – Walloping

This is not the place for this discussion. Contact me by chat and I will explain why your solution doesn't work. – Peewit

See @Chris Knight 's answer for the reason why Visitor can not be applied in some cases. – Irriguous

@Walloping It doesn't solve anything if the decision has to be made only upon the type of the object that is accepting the visitor. The thing here is that types also carry an information upon which a decision can be made and adding additional information for the same decision is redundant and can lead to needless memory overhead. – Kho

I've been looking for this answer and found several on SO where the visitor pattern is suggested. Missing the point of the question! It seems the visitor pattern is greatly misunderstood. – Melody

@Walloping The point of question that you can't change class that should be visited, and you can't add accept visitor method – Woolsey

Answer 2

As highlighted in the comments, the visitor pattern would be a good choice. But without direct control over the target/acceptor/visitee you can't implement that pattern. Here's one way the visitor pattern could possibly still be used here even though you have no direct control over the subclasses by using wrappers (taking Integer as an example):

public class IntegerWrapper {
    private Integer integer;
    public IntegerWrapper(Integer anInteger){
        integer = anInteger;
    }
    //Access the integer directly such as
    public Integer getInteger() { return integer; }
    //or method passthrough...
    public int intValue() { return integer.intValue(); }
    //then implement your visitor:
    public void accept(NumericVisitor visitor) {
        visitor.visit(this);
    }
}

Of course, wrapping a final class might be considered a smell of its own but maybe it's a good fit with your subclasses. Personally, I don't think instanceof is that bad a smell here, especially if it is confined to one method and I would happily use it (probably over my own suggestion above). As you say, its quite readable, typesafe and maintainable. As always, keep it simple.

Herbst

Yes, "Formatter", "Composite", "Different types" all seams to point in the direction of visitor. – Draftee

how do you determine which wrapper you are going to use? through a if instanceof branching? – Tidwell

As @fasttooth points out this solution only shifts the problem. Instead of using instanceof to call the right handle() method you'll now have to use it the call the right XWrapper constructor... – Canaliculus

Answer 3

Instead of a huge if, you can put the instances you handle in a map (key: class, value: handler).

If the lookup by key returns null, call a special handler method which tries to find a matching handler (for example by calling isInstance() on every key in the map).

When a handler is found, register it under the new key.

This makes the general case fast and simple and allows you to handle inheritance.

Collegian

+1 I have used this approach when handling code generated from XML schemas, or messaging system, where there are dozens of object types, handed to my code in an essentially non-typesafe way. – Funchal

Answer 4

You can use reflection:

public final class Handler {
  public static void handle(Object o) {
    try {
      Method handler = Handler.class.getMethod("handle", o.getClass());
      handler.invoke(null, o);
    } catch (Exception e) {
      throw new RuntimeException(e);
    }
  }
  public static void handle(Integer num) { /* ... */ }
  public static void handle(BigDecimal num) { /* ... */ }
  // to handle new types, just add more handle methods...
}

You can expand on the idea to generically handle subclasses and classes that implement certain interfaces.

Iaea

I would argue that this smells even more then the instanceof operator. Should work though. – Genny

@Tim Büthe: At least you don't have to deal with a growing if then else chain in order to add, remove or modify handlers. The code is less fragile to changes. So I'd say that for this reason it's superior to the instanceof approach. Anyway, I just wanted to give a valid alternative. – Spooner

This is essentially how a dynamic language would handle the situation, via duck typing – Funchal

@DNA: wouldn't that be multimethods? – Spooner

@Iaea Oops, yes! I shouldn't post so late at night ;-) – Funchal

Why do you iterate over all methods instead of using getMethod(String name, Class<?>... parameterTypes)? Or else I would replace == with isAssignableFrom for the parameter's type check. – Hydrangea

@AleksandrDubinsky: you're right, it would be better to call it directly like that. I'll check later and change it accordingly. – Spooner

Adding reflection to production code can cause drastic results as Reflection is comparatively lot slower depending upon the operation. Please read: https://mcmap.net/q/75771/-java-reflection-performance – Keeper

@DipeshGupta: you're absolutely right, I think that should go without saying. – Spooner

Answer 5

I think that the best solution is HashMap with Class as key and Handler as value. Note that HashMap based solution runs in constant algorithmic complexity θ(1), while the smelling chain of if-instanceof-else runs in linear algorithmic complexity O(N), where N is the number of links in the if-instanceof-else chain (i.e. the number of different classes to be handled). So the performance of HashMap based solution is asymptotically higher N times than the performance of if-instanceof-else chain solution. Consider that you need to handle different descendants of Message class differently: Message1, Message2, etc. . Below is the code snippet for HashMap based handling.

public class YourClass {
    private class Handler {
        public void go(Message message) {
            // the default implementation just notifies that it doesn't handle the message
            System.out.println(
                "Possibly due to a typo, empty handler is set to handle message of type %s : %s",
                message.getClass().toString(), message.toString());
        }
    }
    private Map<Class<? extends Message>, Handler> messageHandling = 
        new HashMap<Class<? extends Message>, Handler>();

    // Constructor of your class is a place to initialize the message handling mechanism    
    public YourClass() {
        messageHandling.put(Message1.class, new Handler() { public void go(Message message) {
            //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message1
        } });
        messageHandling.put(Message2.class, new Handler() { public void go(Message message) {
            //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message2
        } });
        // etc. for Message3, etc.
    }

    // The method in which you receive a variable of base class Message, but you need to
    //   handle it in accordance to of what derived type that instance is
    public handleMessage(Message message) {
        Handler handler = messageHandling.get(message.getClass());
        if (handler == null) {
            System.out.println(
                "Don't know how to handle message of type %s : %s",
                message.getClass().toString(), message.toString());
        } else {
            handler.go(message);
        }
    }
}

More info on usage of variables of type Class in Java: http://docs.oracle.com/javase/tutorial/reflect/class/classNew.html

Karlotta

for a small number of cases (probably higher than the number of those classes for any real example) the if-else would outperform the map, besides of no using heap memory at all – Sparks

Answer 6

You could consider the Chain of Responsibility pattern. For your first example, something like:

public abstract class StuffHandler {
   private StuffHandler next;

   public final boolean handle(Object o) {
      boolean handled = doHandle(o);
      if (handled) { return true; }
      else if (next == null) { return false; }
      else { return next.handle(o); }
   }

   public void setNext(StuffHandler next) { this.next = next; }

   protected abstract boolean doHandle(Object o);
}

public class IntegerHandler extends StuffHandler {
   @Override
   protected boolean doHandle(Object o) {
      if (!o instanceof Integer) {
         return false;
      }
      NumberHandler.handle((Integer) o);
      return true;
   }
}

and then similarly for your other handlers. Then it's a case of stringing together the StuffHandlers in order (most specific to least specific, with a final 'fallback' handler), and your despatcher code is just firstHandler.handle(o);.

(An alternative is to, rather than using a chain, just have a List<StuffHandler> in your dispatcher class, and have it loop through the list until handle() returns true).

Schweiker

Answer 7

Just go with the instanceof. All the workarounds seem more complicated. Here is a blog post that talks about it: http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html

Frowst

Answer 8

I have solved this problem using reflection (around 15 years back in pre Generics era).

GenericClass object = (GenericClass) Class.forName(specificClassName).newInstance();

I have defined one Generic Class ( abstract Base class). I have defined many concrete implementations of base class. Each concrete class will be loaded with className as parameter. This class name is defined as part of configuration.

Base class defines common state across all concrete classes and concrete classes will modify the state by overriding abstract rules defined in base class.

At that time, I don't know the name of this mechanism, which has been known as reflection.

Few more alternatives are listed in this article : Map and enum apart from reflection.

Dominoes

Just curious, why didn't you make GenericClass an interface? – Comprador

I had common state and default behaviour, which has to be shared across many related objects – Dominoes

Answer 9

Add a method in BaseClass which returns name of the class. And override the methods with the specific class name

public class BaseClass{
  // properties and methods
  public String classType(){
      return BaseClass.class.getSimpleName();
  }
}

public class SubClass1 extends BaseClass{
 // properties and methods
  @Override
  public String classType(){
      return SubClass1.class.getSimpleName();
  }
}

public class SubClass2 extends BaseClass{
 // properties and methods
  @Override
  public String classType(){
      return SubClass1.class.getSimpleName();
  }
}

Now use the switch case in following way-

switch(obj.classType()){
    case SubClass1:
        // do subclass1 task
        break;
    case SubClass2:
        // do subclass2 task
        break;
}

Mange

Answer 10

What I use for Java 8:

void checkClass(Object object) {
    if (object.getClass().toString().equals("class MyClass")) {
    //your logic
    }
}

Aeciospore