I am currently working on a project where I need to manipulate a tree-like data structure, which I implemented with a composite pattern. I want to be able to do different actions on this data structure, so I implemented a visitor pattern.

I was starting to see some code duplication in between my visitors, so I decided to implement a decorator pattern over my visitors. The decorators implement the Visitor interface, and their constructor accepts a Visitor as a parameter.

enter image description here

The idea is that the decorator executes some code before, executes its base visitor (the one passed as a parameter), and executes some code after, a bit like that:

class ConcreteDecorator {
    decoratedVisitor: Visitor

    visitLeaf(leaf) {
        // Do some stuff before
        result := decoratedVisitor.visitLeaf(leaf)
        // Do some stuff after
        return result;
    }

    visitComposite(composite) {
        // Do some stuff before
        result := decoratedVisitor.visitComposite(leaf)
        // Do some stuff after
        return result;
    }
}

The decorator/visitor stuff can be setup like that:

visitor := new ConcreteVisitorA()
visitor := new ConcreteDecoratorA(visitor)
Visitor := new ConcreteDecoratorB(visitor)

The problem is that once I call visitComposite on a decorator, for example, the decorator does its stuff, then calls its child visitor's visitComposite. Since visitComposite is a recursive method, when it is called on the ConcreteVisitor and the method does its recursive calls, we lose the decorators. The decorators are only applied to the first visited node.

I thought of different solutions, like using inheritance instead of decorators, but I want to be able to reuse the decorators on different visitors, so that's not a viable option.

I also thought of using some kind of strategy pattern. The strategies would have a beforeVisitLeaf/afterVisitLeaf and beforeVisitComposite/afterVisitComposite methods. The problem is I can't implement a filter decorator, which abort the visit of certain nodes.

I managed to do a hack this solution and make it work by keeping a hook on the child visitor methods and substituting the child visitor instance methods by the decorator methods manually (I can do that because... JavaScript).

This solution is super hacky, so I was wondering if you guys had any design ideas on how to fix this.

New contributor
Antoine Boisier-Michaud is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.

The fix is to remove the composite.

I mean, you can certainly keep it for the structure, but the entire point of the composite is that things can operate against some composite ignorant interface, and if it happens to be a composite then it works against multiple things.

Because the visitors know about the implementations of that interface it's not really a composite. You lose a ton of benefit and run into problems like the one you've got here.

Instead I'd not treat it as a composite. Either have a single class with children sometimes be empty (meaning you don't even need a visitor), or treat leaf and composite node as plain old data. Visitors are applied to a node but not expected to apply themselves to its children (but maybe need to look at them to make a decision).

Effectively, take a little more functional approach where data and behavior are distinct.

  • Thanks for your answer, @Telastyn! The trees I work with tend to be a few level deep, but very wide. This means I would have a lot of empty fields in my class. I don't think this is a good solution for my specific use case, but your answer is definitely interesting. – Antoine Boisier-Michaud Dec 7 at 20:44
  • @AntoineBoisier-Michaud - what? How does the width of your tree (number of children) have anything to do with the number of fields the nodes have? – Telastyn Dec 8 at 0:19
  • Sorry, my comment definitely unclear. The way I understood your answer, you suggested getting rid of the leaf class and having only a node class. The problem is that my composite class and my leaf class have different fields. This means that, not only the leaf nodes would have zero children, they would also have useless fields that are specific to composite nodes. – Antoine Boisier-Michaud Dec 8 at 4:40
  • 1
    @AntoineBoisier-Michaud - then go the other route. Use the visitor, but don't have the visitors walk the tree. – Telastyn Dec 8 at 5:14

From your description, I get the feeling that your code looks similar to this

class Composite 
{
    accept<T>(Visitor<T> visitor) {
        visitor.visitComposite(this)
    }
}

class ConcreteVisitor // inherits Visitor<T>
{
    visitComposite(composite) {
        // do some work
        for child in composite.children {
            child.accept(this)
        }
        // do some more work
    }
}

If that is the case, then the root of your problem lies in the fact that you moved a responsibility of the Composite class (from the Composite pattern) into the visitComposite function of your visitor.
When calling a method of a Composite class of the Composite pattern, it is the responsibility of that class to forward the call to the child nodes as needed.

If you don't want to tie your Visitor pattern to a particular traversal order of composites and children of the Composite pattern, then you can extend your visitor classes to have two functions that should be called by the Composite class, like this

class Composite 
{
    accept<T>(Visitor<T> visitor) {
        visitor.visitCompositePreChildren(this)
        for child in children {
            child.accept(visitor)
        }
        visitor.visitCompositePostChildren(this)
    }
}

class ConcreteVisitor // inherits Visitor<T>
{
    visitCompositePreChildren(composite) {
        // do some work before the child nodes
    }
    visitCompositePostChildren(composite) {
        // do some work after the child nodes
    }
}

This also makes your decorator pattern on the visitors possible.

  • Thank you for your answer Bart! Your feeling is right, my implementation does look like that. Your solution partially solves my problem. I still run into the problem where my visitor can't return before visiting a node. I guess this isn't really the visitor's job though. – Antoine Boisier-Michaud Dec 7 at 20:58
  • @AntoineBoisier-Michaud: Your visitor can't know which nodes are really present in the structure it is visiting. The best it can do is remember that it has visited NodeA and then take action on that later (for example when all children of NodeA's parent have been visited or when NodeB is being visited). – Bart van Ingen Schenau Dec 8 at 9:16

Your Answer

Antoine Boisier-Michaud is a new contributor. Be nice, and check out our Code of Conduct.

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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