A Variation on Visitor Pattern: Why not move the 2nd dispatch into the visitor's `Visit` method?

Intro

Apparently, I've been doing an "unorthodoxed" Visitor pattern my whole programmer life.

Yes, I dispatch to a concrete composite element visit method from the Visitor's Visit method.

I think this is how I learned it, but now I can't find any examples of it, and the source I learned it from is gone.

Now, faced with overwhelming evidence that the concrete element dispatch goes into the composite element's Accept method, I'm wondering if the way I had been doing it has at least some advantage. The two advantages that appear to me are:

1. I have a single place to make the decision of how to dispatch: the base visitor.
2. I can add new composite element types, and have the base visitor ignore them, yet a derived visitor could override Visit to handle them.

Examples

Here is the basic Composite/Visitor model:

// "Unorthodox" version
public class BaseVisitor 
{
    public virtual void Visit(CompositeElement e)
    {
         if(e is Foo)
         {
             VisitFoo((Foo)e);
         }
         else if(e is Bar)
         {             
             VisitBar((Bar)e);
         }
         else
         {
             VisitUnknown(e);
         }
    }

    protected virtual void VisitFoo(Foo foo) { }
    protected virtual void VisitBar(Bar bar) { }
    protected virtual void VisitUnknown(CompositeElement e) { }
} 

public class CompositeElement 
{
    public virtual void Accept(BaseVisitor visitor) { } 
}

public class Foo : CompositeElement { }
public class Bar : CompositeElement { }

Note the visitor class is now responsible for the 2nd type-based dispatch, instead of the canonical version, where, for example, Foo would be responsible for it and would have:

// Canonical visitor pattern 2nd dispatch
public override void Accept(BaseVisitor visitor)
{
    visitor.VisitFoo(this);
}

Now, for the defense...

Advantage 1

Let's say we want to add a new CompositeElement type:

public class Baz : CompositeElement { }

In order to accomodate this new element type in the visitor model, I just need to make changes to the BaseVisitor class:

public class BaseVisitor 
{  
    public virtual void Visit(CompositeElement e)
    {
        // Existing cases elided...
        else if(e is Baz)
        {
            VisitBaz((Baz)e);
        }
    }

    protected virtual void VisitBaz(Foo foo) { }
}

Admittedly, this is a small issue, but it does appear to simplify maintenance (that is, if you don't mind big if or switch statements).

Advantage 2

Let's say we want to extend the composite in a separate package. We can accomodate this without modifying BaseVisitor:

public class ExtendedVisitor : BaseVisitor
{
    public override Visit(CompositeElement e)
    {
        if(e is ExtendedElement)
        {
            VisitExtended((ExtendedElement)e);
        }
        else
        {
            base.Visit(e);
        }            
    }

    protected virtual void VisitExtended(ExtendedElement e) { }
}

public class ExtendedCompositeElement : CompositeElement { }

Having this struc开发者_开发知识库ture allows us to break the dependency of BaseVisitor needing to have VisitExtended in order to accomodate extended CompositeElement types.

Conclusion

I haven't implemented Visitor pattern enough or maintained it long enough to have any disadvantages weigh on me at this point. Obviously, maintaining a big switch statement is a pain, and there are performance implications, however I'm not sure they outweigh the flexibility of keeping the BaseVisitor free of dependences on extensions.

Please weigh in with your thoughts on the downsides.

The main reason the visitor pattern is defined in the GoF book as it is, is that C++ didn't have any form of Run-Time Type Identification (RTTI). They used "double dispatch" to get the target objects to tell them what their type was. A pretty cool, but incredibly hard to describe trick.

The main difference between what you describe and the GoF Visitor pattern (as you mention) is that you have an explicit "dispatch" method - the "visit" method that checks the type of the argument and sends it to the explicit visitFoo, visitBar, etc methods.

The GoF Visitor pattern uses the data objects themselves to do the dispatch by providing an "accept" method that turns around and passes "this" back to the visitor, resolving to the proper method.

To put it all in one place, the basic GoF pattern looks like (I'm a Java guy, so please excuse the Java code instead of C# here)

public interface Visitor {
    void visit(Type1 value1);
    void visit(Type2 value2);
    void visit(Type3 value3);
}

(note that this interface could be a base class with default method implementations if you would like)

and your data objects all need to implement an "accept" method:

public class Type1 {
    public void accept(Visitor v) {
        v.visit(this);
    }
}

Note: The big difference between this and what you mentioned for the GoF version is that we can use method overloading so the "visit" method name stays consistent. This allows every data object to have an identical implementation of "accept", reducing the chance of a typo

Every type needs the exact same method code. The "this" in the accept method causes the compiler to resolve to the correct visit method.

You can then implement the Visitor interface however you would like.

Note that adding a new type (Type4 for example), in the same or different package, would require fewer changes than what you describe. If in the same package, we would add a method to the Visitor interface (and each implementation), but you don't need the "dispatch" method.

That said...

• The GoF implementation requires the cooperation/modification of the data objects. This is the main thing I don't like about it (aside from trying to describe it to someone, which can be quite painful. Lots of people have trouble with the "double dispatch" concept). I vastly prefer to keep my data and what I'm going to do with it separate - MVC type approach.
• Both your implementation and the GoF implementation require code changes to add new types - this can break existing visitor implementations
• Both your implementation and the GoF implementation are static; the "what to do" for specific types cannot be changed at runtime
• We now have RTTI in the languages that are used most often

I teach Design Patterns at Johns Hopkins, by the way, and what I like to recommend is a nicely dynamic approach.

Start with a simpler, single-object Visitor interface:

public interface Visitor<T> {
    void visit(T type);
}

Then create a VisitorRegistry

public class VisitorRegistry {
    private Map<Class<?>, Visitor<?>> visitors = new HashMap<Class<?>, Visitor<?>>();
    public <T> void register(Class<T> clazz, Visitor<T> visitor) {
        visitors.put(clazz, visitor);
    }
    public <T> void visit(T thing) {
        // needs error checks, and possibly "walk up" to check supertypes if direct type not found
        // also -- can provide default action to perform - maybe register using Void.class?
        @SuppressWarnings("unchecked")
        Visitor<T> visitor = (Visitor<T>) visitors.get(thing.getClass());
        visitor.visit(thing);
    }
}

You would use this like

VisitorRegistry registry = new VisitorRegistry();
registry.register(Person.class, new Visitor<Person>() {
    @Override public void visit(Person person) {
        System.out.println("I see " + person.getName());
    }});
// register other types similarly

// walk the data however you would...
for (Object thing : things) {
    registry.visit(thing);
}

This allows you to now register independent visitors for each type you want to visit, and it won't break existing visitor implementations whenever a new type is added.

You can also re-register (and de-register) different combinations of visitors at runtime, even loading definitions of what to do from some configuration info.

Hope this helps!

Take a look at the acyclic visitor pattern. It also offers the advantages that you listed in your visitor adaptation, without the big switch statement:

// acyclic version 
public interface IBaseVisitor { }
public interface IBaseVisitor<T> : IBaseVisitor where T : CompositeElement {
  void Visit(T e) { }
}
public class CompositeElement {
  public virtual void Accept(IBaseVisitor visitor) { }
}
public class Foo : CompositeElement {
  public override void Accept(IBaseVisitor visitor) {
    if (visitor is IBaseVisitor<Foo>) {
      ((IBaseVisitor<Foo>)visitor).Visit(this);
    }
  }
}
public class Bar : CompositeElement {
  public override void Accept(IBaseVisitor visitor) {
    if (visitor is IBaseVisitor<Bar>) {
      ((IBaseVisitor<Bar>)visitor).Visit(this);
    }
  }
}

Your real visitors can pick and choose which subclasses they visit:

public class MyVisitor : IBaseVisitor<Foo>, IBaseVisitor<Bar> {
  public void Visit(Foo e) { }
  public void Visit(Bar e) { }
}

It's "acyclic" because it doesn't have a cyclic dependency between the types in your hierarchy and the methods in the visitor.

Apart from the downsides you mention already (performance and the need to maintain a big switch statement), another problem is that with the GoF Visitor pattern, adding a new subclass of CompositeElement will force you to write a handler for it or your code won't even compile. With your approach on the other hand, it would be easy to add new CompositeElement subclasses and forget to update the appropriate visitor switch statements.

Your suggestion of subclassing visitors, handling only a subset of classes in certain visitors, makes this even worse. Now when a developer creates a new subclass of CompositeElement, they will need intimate knowledge of all existing visitor classes in order to know which ones do and which do not need to be changed, which will be very easy to get wrong.

Some languages also have restrictions that make this very unattractive. Java doesn't have multiple inheritence except through interfaces. Requiring every composite element and visitor to derive from the same base class would make for a gross type hierarchy.

i.e. your way doesn't allow Visitor and CompositeElement to be interfaces.

I don't like implementations with visitA, visitB, visitWhatever, acceptA, acceptB, acceptWhatever because this approach implies you will be breaking interfaces everytime you add a class to your hierarchy.

Please have a look at an article I've written about this.

The article explains it all in detail, using real life examples, including a polymorphic case which does not break any interface.