Modular Visitor Pattern

Motivation

I created this because the Visitor pattern felt like the most appropriate, but I also wanted to satisfy the Open/Closed principle with respect to the logic when visiting specific node types. I also needed the ability to "return" a specialized value type from visitation without wanting to tie that knowledge to the visitable hierarchy. Why should those objects have to care what I'm making out of them?

The Visitor/Visitable classes are defined mostly as by Alexandrescu.

[Note: The existing codebase uses Qt, so I left the Qt classes intact rather than switch them for std equivalents.]

Visitor/Visitable Classes:

// Strawman base class
class BaseVisitor
{
public:
    virtual ~BaseVisitor() {};
};

// Class template for visitor, template params are type visited and return type of visit
template 
class Visitor
{
public:
    virtual R visit(const T &) = 0;
};

// Class template for visitable, template param is the return type of accept

template 
class BaseVisitable
{
public:
    virtual ~BaseVisitable() {};
    virtual R accept(BaseVisitor &)
    {
        return (R) 0;
    }

    typedef R ReturnType;
protected:
    template 
    static ReturnType acceptVisitor(const T &visited, BaseVisitor &visitor)
    {
        if (typeid(visited) == typeid(T))
        {
            auto p = dynamic_cast *> (&visitor);
            // We want to ignore non-specified nodes
            if (p != nullptr)
                return p->visit(visited);
            else
                return (ReturnType) 0;
        }
        else
        {
            throw std::runtime_error("typeid(T) doesn't match typeid(visited)! Did your inherited class miss DEFINE_VISITABLE()?");
        }
    }

};

#define DEFINE_VISITABLE() \
            virtual ReturnType accept(BaseVisitor &v) const \
                { return acceptVisitor(*this, v); }

Visitable Hierarchy

```
class VisitableBase : public BaseVisitable
{
public:
DEFINE_VISITABLE();
};

class VisitableDerived : public Visit

The visitor pattern and open closed principle. There is a mild conflict here.

The visitor pattern is very good when you have a static class hierarchy and just want different ways to traverse the nodes in some collection. In these situations the visitor pattern behaves very well.

BUT

If the class hierarchy is not stable and you are adding new types of nodes then it does not behave very well. This is because every new class that you add to the hierarchy will require a new method in the visitor object (unless you want to default them using some template method to handle the new nodes).

Design

Not sure I agree with your design at all.

This is what a standard visitor pattern looks like.

class A;
 class B;
 class C;
 class D;
 class E;
 class AlphaVisitor
 {
      public:
          virtual void visit(A& a) = 0;
          virtual void visit(B& b) = 0;
          virtual void visit(C& c) = 0;
          virtual void visit(D& d) = 0;
          virtual void visit(E& e) = 0;
 };
 class A {public: accept(AlphaVisitor& v) {/* STUFF*/ v.visit(*this);}};
 class B {public: accept(AlphaVisitor& v) {/* STUFF*/ v.visit(*this);}};
 class D {public: accept(AlphaVisitor& v) {/* STUFF*/ v.visit(*this);}};
 class E {public: accept(AlphaVisitor& v) {/* STUFF*/ v.visit(*this);}};

In your code both accept() and visit() return an R. This is not normal. It may be very big object that is returned. You don't want to copy that around. It is more common for the AlphaVisitor to accumulate state as it visits objects. Then when it has finished visiting then you can access the state from the visitor object itself.

A                a(/* construct some large graph of A/B/C/D/E */);

 MyDerivedVisitor visitor;
 a.accept(visitor);

 ResultType value = visitor.getResult();

Code Review

R had better be constructible with an integer. Otherwise this will fail to even compile.

virtual R accept(BaseVisitor &)
{
    return (R) 0;
}

Runtime type checking is not a good idea. That is why we have the visitor pattern to prevent this.

static ReturnType acceptVisitor(const T &visited, BaseVisitor &visitor)
{
    if (typeid(visited) == typeid(T))
    {
    }
    else
    {
        throw std::runtime_error("typeid(T) doesn't match typeid(visited)! Did your inherited class miss DEFINE_VISITABLE()?");
    }
}

Please don't do this:

auto newBuilder1 = new ConverterVisitor();

auto newBuilder2 = new ConverterVisitor();

auto newBuilder3 = new ConverterVisitor();

Just declare your variables locally.

ConverterVisitor   newBuilder1;
ConverterVisitor  newBuilder2;

ConverterVisitor   newBuilder3;

The same as above

VisitableDerived  visitable;
VisitableDerived2 visitable2;

visitable.accept(newBuilder1);
visitable2.accept(newBuilder1);

StackExchange Code Review Q#153786, answer score: 4