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.

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 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.

2 Answers
2

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 compositenode 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-MichaudDec 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?
– TelastynDec 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-MichaudDec 8 at 4:40

1

@AntoineBoisier-Michaud - then go the other route. Use the visitor, but don't have the visitors walk the tree.
– TelastynDec 8 at 5:14

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

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-MichaudDec 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 SchenauDec 8 at 9:16