Instanceof correct usage in java

Question

Instance of is considered to be a bad practice in java. But I don't know how to avoid it in such situation. Let's say I have an abstract class ArtObject and there are some subclasses such as Painting, Engraving, etc. And I have a class Museum which contains an ArrayList of ArtObjects.

For example, I'd like to print information about all Painting objects in the Museum.

Should I do something like this?:

public void printAllPaintingsInfo() {
    for (int i = 0; i < artObjects.size(); i++) {
        if (artObjects.get(i) instanceof Painting) {
            System.out.println(artObjects.get(i));
        }
    }
}

Or a better approach exists?

Answer 1

As with many language features that are usually frowned upon, it is often tempting to cheat around using them by implementing a mechanism that somehow “fakes” them. But this doesn't make anything better. If you mean to test the type of an object, then instanceof is just the right thing to use. If you want to avoid it, you'll need to find a way that does not require you to find out the (dynamic) type of an object.

One popular pattern to avoid run-time type inspection is using the visitor pattern which effectively has the overload resolution and dynamic dispatch machinery do the dirty work. If your real-world problem is as simple as the museum example you've posted, then this is clearly overkill. Anyway, here is how it could be done.

For the sake of this example, assume we have Paintings and Drawings, both derived from ArtObject. We define an abstract visitor type that has a method for each ArtObject. Alternatively, you can only specialize for a few and have a fallback “visit anything” method.

public abstract class ArtVisitor {

    public void visit(final Painting painting) {
    }

    public void visit(final Drawing drawing) {
    }

    // public void visit(final ArtObject anything) {
    // }
}

I have used an abstract class instead of an interface so I can have empty default implementations of the visit methods.

In the ArtObject interface (or abstract class, for that matter) we need to add an abstract takeVisitor method.

public abstract class ArtObject {

    private final String name;

    protected ArtObject(final String name) {
        this.name = name;
    }

    public abstract void takeVisitor(ArtVisitor visitor);

    @Override
    public final String toString() {
        return String.format("%s (%s)",
                             this.name,
                             this.getClass().getCanonicalName());
    }
}

Unfortunately, we cannot implement ArtObject.takeVisitor since the this pointer would have static type ArtVisitor, which would require us to do type introspection which is just what we wanted to avoid. Instead, we must – and this is the ugliest thing about the visitor pattern – override it in each of our classes.

(If we had added the fallback ArtVisitor.visit(ArtObject) method, we could have implemented a generic takeVisitor method but we'd still have to override it in our derived classes to make overload resolution based on the type of the this pointer work. In most cases, this would add more confusion (and potential bugs) than benefits.)

public final class Painting extends ArtObject {

    public Painting(final String name) {
        super(name);
    }

    @Override
    public void takeVisitor(final ArtVisitor visitor) {
        visitor.visit(this);
    }

    public void aMethodOnlyPaintingsHave() {
        // whatever...
    }
}

public final class Drawing extends ArtObject {

    public Drawing(final String name) {
        super(name);
    }

    @Override
    public void takeVisitor(final ArtVisitor visitor) {
        visitor.visit(this);
    }
}

Now we can build the museum in a straight-forward manner.

import java.util.List;
import java.util.ArrayList;

public final class Museum {

    private final List<ArtObject> artworks;

    public Museum() {
        this.artworks = new ArrayList<ArtObject>();
        artworks.add(new Painting("Mona Lisa"));
        artworks.add(new Drawing("Madame Palmyre with Her Dog"));
        artworks.add(new Painting("The Night Watch"));
    }

    public void printAllWorks() {
        for (final ArtObject work : this.artworks) {
            System.out.println(work);
        }
    }

    public void printAllPaintings() {
        final ArtVisitor paintingsPrinter = new ArtVisitor() {

            @Override
            public void visit(final Painting painting) {
                System.out.println(painting);
                // Note: We don't need any unsafe downcast here!
                painting.aMethodOnlyPaintingsHave();
            }
        };
        for (final ArtObject work : this.artworks) {
            work.takeVisitor(paintingsPrinter);
        }
    }

    public static void main(final String[] args) {
        final Museum myMuseum = new Museum();
        System.out.println("All ArtObjects:\n");
        myMuseum.printAllWorks();
        System.out.println("\n\nAll Paintings:\n");
        myMuseum.printAllPaintings();
    }
}

The output of the above program is:

All ArtObjects:

Mona Lisa (Painting)
Madame Palmyre with Her Dog (Drawing)
The Night Watch (Painting)


All Paintings:

Mona Lisa (Painting)
The Night Watch (Painting)