How to find type without using instanceof?

I have a List of interface type Criter开发者_StackOverflowia within my class Query.

List<Criteria> criteria = new ArrayList<Criteria>();

I have several concrete implementations of Criteria. I want to give Query a method that iterates through my criteria list, and depending on the concrete type, execute some logic.

I'm presently doing this with instanceof like so:

for(Criteria c : criteria) {
    if(c instanceof ContextualCriteria){
        // logic
    }
    ...
}

Is this the only/best way?

Does the logic sensibly belong in the Criteria itself? If so, put it into the Criteria interface and implement it appropriately for each concrete class implementing the Criteria interface. This is obviously the nice polymorphic approach.

Unfortunately, in real life OO doesn't always work as simply as that - sometimes it doesn't make sense to put the per-type behaviour in the type itself, so you may need to use instanceof instead. You could potentially have a map from "criteria class" to some interface representing the action to take, but that could easily end up being even messier.

Double-dispatch via the visitor pattern can sometimes improve things a little - so the logic could still be in separate methods in your "calling" class, but each Criteria can call back to the right method via a single interface method. Personally I tend to find this increases coupling and gets ugly quickly, but others swear by it.

If the logic does not belong in the Criteria as Jon Skeet suggests, then you could use the visitor pattern.

In ConcreteCriteria:

public void accept(CriteriaVisitor v) {
    v.visit(this);
}

In the Client code:

public void method() {
    for (Criteria c : criteria) {
        c.accept(this);
    }
}

public void visit(ConcreteCriteria c) {
    // do logic here
}

public void visit(Criteria c) {
    // othervise...
}

This gets rid of the instanceof, but be wary, I have found that this pattern is difficult to understand if you are unfamiliar with the code.

Well, you can...

if (ContextualCriteria.class.equals(c.getClass()) {

... though it's just a fancier-looking way of writing instanceof. (Well, almost: this tests whether it is exactly the class, rather than the class of a subclass -- for that you want isAssignableFrom()).

The right way to get rid of the smell is to implement a polymorphic method in Criteria which is overridden in subclasses, for example.

An interface is halfway to the strategy pattern! To vary the logic based on type, push it behind the interface if possible, such that Criteria has a doLogic(). You can pass that method whatever parameters you might need to alter in the calling code, or return new information - that is very implmentation specific and hard to advice on from the code in question.

If all goes well, your calling code ends up

for (Criteria c : criteria) {
    c.doLogic();
}

You could also implement the logic in the classes which implement Criteria

public interface Criteria {
  public void logic();
}

Have several implemetations in classes like ContextualCriteria And your loop would look clean:

for(Criteria c : criteria) {  
   c.logic();
}  

You can also use:

for(Criteria c : criteria) {
   if(c.getClass() == ContextualCriteria.class){
       // logic
   }
   if ...
}

Note that Object#getClass() returns the runtime type of c, so you can't reliably use this if ContextualCriteria can be subclassed for example. To do this you would need to use Class#isAssignableFrom():

for(Criteria c : criteria) {
   if(ContextualCriteria.class.isAssignableFrom(c)){
       // logic
   }
   if ...
}

There is nothing wrong with it.

People who never used instanceof only wrote toy applications.