Using visitor pattern instead of casting

Question

I make regular use of the visitor pattern in my code. When a class hierarchy has a visitor implemented, I use it as an alternative to instanceof and casting. However it leads to some pretty awkward code which I'd like to improve.

Consider the contrived case:

interface Animal {
    void accept(AnimalVisitor visitor);
}

class Dog implements Animal {
    void accept(AnimalVisitor visitor) {
        visitor.visit(this);
    }
}

class Cat implements Animal {
    void accept(AnimalVisitor visitor) {
        visitor.visit(this);
    }
}

interface AnimalVisitor {
    default void visit(Cat cat) {};
    default void visit(Dog dog) {};
}

In the majority of cases, to do something specific to dogs only (for example) I implement a visitor that implements the logic in its visit method - just as the pattern intends.

There are case, however, in which I want to return an optional dog from the visitor to use outside.

In these case I end up with some pretty ugly code:

List<Dog> dogs = new ArrayList<>();
animal.accept(new AnimalVisitor() {
    void visit(Dog dog) {
        dogs.add(dog);
    }
}
Optional<Dog> possibleDog = dogs.stream().findAny();

I can't assign possibleDog directly inside the visitor because it's not a final variable, hence the list.

This is pretty ugly and inefficient just to get around requirement for effective finality. I'd be interested in ideas of alternatives.

Alternatives I've considered:

Turning the visitor into a generic which can be given a return value

interface Animal {
    <T> T accept(AnimalVisitor<T> visitor);
}

interface AnimalVisitor <T> {
    default Optional<T> visit(Dog dog) { return Optional.empty(); }
    default Optional<T> visit(Cat cat) { return Optional.empty(); }
}

Creating an abstract visitor that contains most of the code and can be trivial extended to set the optional directly

abstract class AnimalCollector implements AnimalVisitor <T> {
    private Optional<T> result = Optional.empty;

    protected void setResult(T value) {
        assert !result.isPresent();
        result = Optional.of(value);
    }

    public Optional<T> asOptional() {
        return result;
    }
}

Use a stream builder instead of a list

Stream.Builder<Dog> dogs = Stream.builder();
animal.accept(new AnimalVisitor() {
    void visit(Dog dog) {
        dogs.accept(dog);
    }
}
Optional<Dog> possibleDog = dogs.build().findAny();

But I don't find these particularly elegant. They involve a lot of boilerplate just to implement basic asA logic. I tend to use the second solution in my code to keep the usage clean. Is there a simpler solution I'm missing?

Just to be clear, I'm not that interested in answers with some variant of "use instanceof and casts". I realise it would work in this trivial case but the situations I'm considering have quite complex use of visitors that include visiting composites and delegates which make casting impractical.

Answer 1

I'd try factoring the logic using Listeners managed by the visitor. When the visitor finds a dog it wants to return it calls all registered listeners to report the dog. The listener, in turn would be responsible for doing whatever you want to do with the optionally reported dog(s). This may keep your implementation cleaner by moving the reported dog aggregation logic out of your visitor implementation.

Answer 2

In fact, you do not need a AnimalVisitor that return Void. Based on your code and maintaining the Visitor pattern, I would do it the following way.

The Animal interface.

public interface Animal {

    default <T extends Animal> Stream<T> accept(AnimalVisitor visitor) {
        try {
            return visitor.visit(this).stream();
        } catch (ClassCastException ex) {
            return Stream.empty();
        }
    }
}

The Dog and Cat derived classes.

public class Dog implements Animal {

    @Override
    public String toString() {
        return "Fido";
    }
}

public class Cat implements Animal {

    @Override
    public String toString() {
        return "Felix";
    }
}

The AnimalVisitor interface.

public interface AnimalVisitor<T extends Animal> {

    Optional<T> visit(T animal);
}

And putting it all together.

public class AnimalFinder {

    public static void main(String[] args) {

        Animal dog = new Dog();
        Animal cat = new Cat();

        /*
         * The default/old way
         * AnimalVisitor<Dog> dogFinder = new AnimalVisitor<Dog>() {
         *    @Override
         *    public Optional<Dog> visit(Dog animal) {
         *        return Optional.of(animal);
         *    }
         * };
         *
         * Or lambda expression
         *
         * AnimalVisitor<Dog> dogFinder = (animal) -> Optional.of(animal);
         *
         * Or member reference
         */
        AnimalVisitor<Dog> dogFinder = Optional::of;

        Optional fido = dog.accept(dogFinder).findAny();
        Optional felix = cat.accept(dogFinder).findAny();

        System.out.println(fido); // Optional[Fido]
        System.out.println(felix); // Optional.empty

        felix.ifPresent(a -> System.out.printf("Found %s\n", a));
        fido.ifPresent(a -> System.out.printf("Found %s\n", a)); // Found Fido
    }
}

Answer 3

I know you explicitly asked for a solution not using instanceof or cast but actually I think in this special case where you want to implement logic to filter for a specific sub-type this might be worth considering and similar to your generics approach, this is not ugly IMHO:

// as mentioned in the comment above I removed the Optional return types
interface AnimalVisitor<T> {
    T visit(Dog dog);

    T visit(Cat cat);
}

public class AnimalFinder<A extends Animal> implements AnimalVisitor<A> {

    final Class<A> mAnimalClass;

    public AnimalFinder(Class<A> aAnimalClass) {
        this.mAnimalClass = aAnimalClass;
    }

    @Override
    public A visit(Dog dog) {
        if (dog != null && mAnimalClass.isAssignableFrom(dog.getClass())) {
            return mAnimalClass.cast(dog);
        } else {
            return null;
        }
    }

    @Override
    public A visit(Cat cat) {
        if (cat != null && mAnimalClass.isAssignableFrom(cat.getClass())) {
            return mAnimalClass.cast(cat);
        } else {
            return null;
        }
    }
}

Now you can simply reuse the AnimalFinder and provide the type you're interested in as argument:

public static void main(String[] args) {
    Animal dog = new Dog();
    Animal cat = new Cat();

    AnimalVisitor<Dog> dogFinder = new AnimalFinder<>(Dog.class);

    System.out.println(Optional.ofNullable(dog.accept(dogFinder)));
    System.out.println(Optional.ofNullable(cat.accept(dogFinder)));

    // using AnimalFinder there is actually no need to implement something like DogPrinter
    // simply use a Consumer or a lambda expression
    Optional.ofNullable(dog.accept(dogFinder)).ifPresent(d -> System.out.println("Found dog" +  d));
    Optional.ofNullable(cat.accept(dogFinder)).ifPresent(d -> System.out.println("Found dog" +  d));
}

From my point of view this solution has some advantages:

• No duplicated code to filter for different animal classes everywhere in the code
• A single place where a new visit method would need to be added if a new Animal type is implemented
• It's easy to use and easy to extend (something that is not always true with the Visitor pattern)

---

Maurice is right of course. You could simply replace the AnimalFinder visitor by a simple Predicate (actually pretty much what Guava's Predicates.instanceOf does):

public static <A, T> Predicate<A> instanceOf(final Class<T> aClass) {
    return a -> (a != null && aClass.isAssignableFrom(a.getClass()));
}

And use it like this to filter the Optional or Stream:

System.out.println(Optional.ofNullable(dog).filter(instanceOf(Dog.class)));

This is even more reusable (as not restricted to Animals only), has less duplicated code and can be used whenever you got an Optional or Stream.

Answer 4

I don't think you need a visitor in this case; it makes the code redundant. I would rather use a selector:

public class AnimalSelector<T extends Animal> {
    private final Class<T> clazz;

    public AnimalSelector(Class<T> clazz) {
        this.clazz = clazz;
    }

    public T select(Animal a) {
        if (clazz.isInstance(a)) {
            return clazz.cast(a);
        } else {
            return null;
        }
    }
}

Now you could write something like:

Dog dog = new AnimalSelector<>(Dog.class).select(animal);
Cat cat = new AnimalSelector<>(Cat.class).select(animal);

Answer 5

How about this:

interface Animal {
    <T> T accept(AnimalVisitor<T> visitor);
}

static class Dog implements Animal {
    public <T> T accept(AnimalVisitor<T> visitor) {
        return visitor.visit(this);
    }
}

static class Cat implements Animal {
    public <T> T accept(AnimalVisitor<T> visitor) {
        return visitor.visit(this);
    }
}

interface AnimalVisitor<T> {
    default T defaultReturn(){ return null; }
    default T visit(Cat cat) { return defaultReturn(); };
    default T visit(Dog dog) { return defaultReturn(); };
    interface Finder<T> extends AnimalVisitor<Optional<T>> {
        @Override default Optional<T> defaultReturn(){ return Optional.empty(); }
    }
    interface CatFinder extends Finder<Cat> {
        @Override default Optional<Cat> visit(Cat cat){
            return Optional.ofNullable(find(cat));
        }
        Cat find(Cat cat);
        CatFinder FIND = c -> c;
    }
    interface DogFinder extends Finder<Dog> {
        @Override default Optional<Dog> visit(Dog dog){
            return Optional.ofNullable(find(dog));
        }
        Dog find(Dog dog);
        DogFinder FIND = d -> d;
    }
}

Then to use it...

Animal a = new Cat();
Optional<Cat> o = a.accept((CatFinder)c -> {
    //use cat
    return c; //or null to get an empty optional;
});

//or if you just want to always return the cat:
Optional<Cat> o = a.accept(CatFinder.FIND);

You could change find to return a boolean if you want, so it's like a predicate.

Answer 6

By splitting the AnimalCollector idea in the question in two, I think we can create something quite succinct.

(These examples are based around the original AnimalVisitor interface in the question - with such as void visit(Cat cat); as the methods).

The 'collect to Optional' part works well extracted to a standalone class:

public class OptionalCollector<T> {
    private Optional<T> result = Optional.empty();

    public void setResult(T value) {
        result = Optional.of(value);
    }

    public Optional<T> asOptional() {
        return result;
    }
}

Another possibility though is a bit of 'visit by lambda' style coding. A few static factory methods enable a visitor to be easily defined without declaring methods in an anonymous inner class.

These are some example factory methods:

import java.util.function.Consumer;

public class AnimalVisitorFactory {
    static AnimalVisitor dogVisitor(Consumer<Dog> dogVisitor) {
        return new AnimalVisitor() {
            @Override
            public void visit(Dog dog) {
                dogVisitor.accept(dog);
            }
        };
    }

    static AnimalVisitor catVisitor(Consumer<Cat> catVisitor) {
        return new AnimalVisitor() {
            @Override
            public void visit(Cat cat) {
                catVisitor.accept(cat);
            }
        };
    }
}

These two parts can then be combined:

import static AnimalVisitorFactory.*;

OptionalCollector<Dog> collector = new OptionalCollector<>();
animal.accept(dogVisitor(dog -> collector.setResult(dog)));
Optional<Dog> possibleDog = collector.asOptional();

---

This is digressing beyond what's needed for the case in the question, but note the idea could be taken a bit further in a fluent API style. With similar dogVisitor() and catVisitor() default methods on the AnimalVisitor interface too, several lambdas could then be chained together to build a more complete visitor.

Answer 7

I have trialled the generic visitor solution. It's actually not too bad - the main ugliness is the use of Optional<Void> when the visitor doesn't need a return value.

I'm still interested in any better alternatives.

public class Visitor {
    interface Animal {
        <T> Stream<T> accept(AnimalVisitor<T> visitor);
    }

    static class Dog implements Animal {
        @Override
        public String toString() {
            return "Fido";
        }

        @Override
        public <T> Stream<T> accept(AnimalVisitor<T> visitor) {
            return visitor.visit(this).stream();
        }
    }

    static class Cat implements Animal {
        @Override
        public String toString() {
            return "Felix";
        }

        @Override
        public <T> Stream<T> accept(AnimalVisitor<T> visitor) {
            return visitor.visit(this).stream();
        }
    }

    interface AnimalVisitor<T> {
        default Optional<T> visit(Dog dog) {
            return Optional.empty();
        }

        default Optional<T> visit(Cat cat) {
            return Optional.empty();
        }
    }

    public static void main(String[] args) {
        Animal dog = new Dog();
        Animal cat = new Cat();
        AnimalVisitor<Dog> dogFinder = new AnimalVisitor<Dog>() {
            @Override
            public Optional<Dog> visit(Dog dog) {
                return Optional.of(dog);
            }
        };
        AnimalVisitor<Void> dogPrinter = new AnimalVisitor<Void>() {
            @Override
            public Optional<Void> visit(Dog dog) {
                System.out.println("Found dog " + dog);
                return Optional.empty();
            }
        };
        System.out.println(dog.accept(dogFinder).findAny());
        System.out.println(cat.accept(dogFinder).findAny());
        dog.accept(dogPrinter);
        cat.accept(dogPrinter);
    }
}