12

Context

I've been using with a hierarchy of objects (an expression tree) a "pseudo" visitor pattern (pseudo, as in it does not use double dispatch) :

 public interface MyInterface
 {
      void Accept(SomeClass operationClass);
 }

 public class MyImpl : MyInterface 
 {
      public void Accept(SomeClass operationClass)
      {   
           operationClass.DoSomething();
           operationClass.DoSomethingElse();
           // ... and so on ...
      }
 }

This design was, however questionnable, pretty comfortable since the number of implementations of MyInterface is significant (~50 or more) and I didn't need to add extra operations.

Each implementation is unique (it's a different expression or operator), and some are composites (ie, operator nodes that will contain other operator/leaf nodes).

Traversal is currently performed by calling the Accept operation on the root node of the tree, which in turns calls Accept on each of its child nodes, which in turn... and so on...

But the time has come where I need to add a new operation, such as pretty printing :

 public class MyImpl : MyInterface 
 {
      // Property does not come from MyInterface
      public string SomeProperty { get; set; }

      public void Accept(SomeClass operationClass)
      {   
           operationClass.DoSomething();
           operationClass.DoSomethingElse();
           // ... and so on ...
      }

      public void Accept(SomePrettyPrinter printer)
      {
           printer.PrettyPrint(this.SomeProperty);
      }
 }    

I basically see two options :

  • Keep the same design, adding a new method for my operation to each derived class, at the expense of maintainibility (not an option, IMHO)
  • Use the "true" Visitor pattern, at the expense of extensibility (not an option, as I expect to have more implementations coming along the way...), with about 50+ overloads of the Visit method, each one matching a specific implementation ?

Question

Would you recommand using the Visitor pattern ? Is there any other pattern that could help solve this issue ?

11
  • 1
    Perhaps a chain of decorators would be more appropriate?
    – MattDavey
    Jun 6, 2012 at 11:28
  • some questions: how do these implementations differ? what is the structure of the hierarchy? and is it always the same structure? do you always need to traverse the structure in the same order?
    – jk.
    Jun 6, 2012 at 11:29
  • @MattDavey : so you would recommand having one decorator per implementation and operation ?
    – T. Fabre
    Jun 6, 2012 at 11:34
  • 2
    @T.Fabre it's hard to tell. There are 50+ implementors of MyInterface.. do all of those classes have a unique implementation of DoSomething and DoSomethingElse? I don't see where your visitor class actually traverses the hierarchy - it looks more like a facade at the moment..
    – MattDavey
    Jun 6, 2012 at 11:45
  • also what version of C# is it. do you have lambdas? or linq? at your disposal
    – jk.
    Jun 6, 2012 at 11:57

1 Answer 1

13

I have been using the visitor pattern to represent expression trees over the course of 10+ years on six large-scale projects in three programming languages, and I am very pleased with the outcome. I found a couple of things that made applying the pattern a lot easier:

Do not use overloads in the interface of the visitor

Put the type into the method name, i.e use

IExpressionVisitor {
    void VisitPrimitive(IPrimitiveExpression expr);
    void VisitComposite(ICompositeExpression expr);
}

rather than

IExpressionVisitor {
    void Visit(IPrimitiveExpression expr);
    void Visit(ICompositeExpression expr);
}

Add an "catch unknown" method to your visitor interface.

It would make it possible for users who cannot modify your code:

IExpressionVisitor {
    void VisitPrimitive(IPrimitiveExpression expr);
    void VisitComposite(ICompositeExpression expr);
    void VisitExpression(IExpression expr);
};

This would let them build their own implementations of IExpression and IVisitor that "understands" their expressions by using run-time type information in the implementation of their catch-all VisitExpression method.

Provide a default do-nothing implementation of IVisitor interface

This would let users who need to deal with a subset of expression types build their visitors faster, and make their code immune to you adding more methods to IVisitor. For example, writing a visitor that harvests all variable names from your expressions becomes an easy task, and the code will not break even if you add a bunch of new expression types to your IVisitor later on.

7
  • 2
    Can you clarify why you say Do not use overloads in the interface of the visitor ? Jun 6, 2012 at 14:21
  • 1
    Can you explain why you don't recommand using overloads ? I read somewhere (on oodesign.com, actually) that it does not really matters whether I use overloads or not. Is there any specific reason why you prefer that design ?
    – T. Fabre
    Jun 6, 2012 at 14:24
  • 2
    @T.Fabre It does not matter in terms of speed, but it does matter in terms of readability. Method resolution in two of the three languages where I implemented this (Java and C#) require a run-time step to choose among the potential overloads, making the code with massive number of overloads a little harder to read. Refactoring the code also becomes easier, because choosing the method you'd like to modify becomes a trivial task. Jun 6, 2012 at 15:39
  • @SnOrfus Please see my answer to T.Fabre above. Jun 6, 2012 at 15:39
  • @dasblinkenlight C# now offers dynamic to let the runtime decide what overloaded method should be used (not at compile time). Is there still any reason why not to use overloading ? Jun 25, 2018 at 4:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.