HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Acyclic Visitor pattern

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
visitorpatternacyclic

Problem

Here is my attempt at implementing the Acyclic visitor pattern from here in C++11.

The aim is to obtain an implementation which is easy to use and avoids all the boilerplate as much as possible. I would also like to keep the use of macros to a minimum, to keep the code readable.

To use it, one makes the visitable base class (say N) inheriting from the class AbstractVisitable, which defines the pure virtual method void accept(Visitor<>&) = 0 (and its const counterpart: void accept(Visitor<>&) const = 0).

The actual visitable classes A1, ..., An (which are subclasses of N), need to use the macro CONCRETE_VISITABLE which implements the accept method.
Finally, an actual visitor is implemented by deriving from Visitor, and implementing the method void visit(Ai&) override {...} for each class A1, ..., An.

There is also the VisitorFunction class, which is used to simplify the common tasks of passing a parameter to the visitor and getting a result from its application.

Visitor.h

```
#ifndef VISITOR_H_
#define VISITOR_H_

#include
#include
#include
#include
#include
#include

template
struct Visitor {
virtual ~Visitor() = default;
};

template
struct Visitor: public virtual Visitor<> {
virtual void visit(T&) = 0;
};

template
struct Visitor: public Visitor, public Visitor... { };

template
using ConstVisitor = Visitor;

struct AbstractVisitable {
virtual ~AbstractVisitable() = default;
virtual void accept(Visitor<>&) = 0;
virtual void accept(Visitor<>&) const = 0;

protected:
template
static inline void acceptImpl(Visitor<>& v, T& p) {
if (auto av = dynamic_cast>(&v))
av->visit(p);
else if (auto av = dynamic_cast>(&v))
av->visit(p);
else
throw std::domain_error(std::string() + typeid(v).name() +
" does not implement Visitor");
}
};

#define CONCRETE_VISITABLE \
void accept(Visitor<>& v) override { \
using TypeOfTh

Solution

-
You can simplify your Visitor-template:

template
struct Visitor : Visitor... {
    virtual ~Visitor() = default;
};

template
struct Visitor : virtual Visitor<> {
    virtual void visit(T&) = 0;
};


It's a shame C++ doesn't have real mixin's, or we would mark Visitor with at least two template-arguments as a pure compile-time fiction.

-
You are adding expensive logic to accept a Visitor wherever you accept a Visitor. Making all Visitor derive from the corresponding Visitor is far cheaper. Just add a specialization:

template 
struct Visitor : Visitor {
    void visit(T& t) override { visit((const T&)t); }
    virtual void visit(const T&) = 0;
};


Unfortunately, adding explicit specializations allowing a Visitor to visit a Derived, where both kinds of visitors might exist, would contravene the pattern's aims. There extra-logic is needed despite the runtime-cost.

-
Visitable bases should have something more in common than that they are visitable, which sharply diminishes the usefulness of a baseclass of all visitable types (meaning it should be a mixin for performance, if we had such).

And a mixin instead of templates or macros for adding the implementations for the accept-functions would also be nice.

Well, let's do the best we can with what we have, the CRTP, to at least diminish the need for even more multiple inheritance:

namespace detail {
    struct dummy {};
    template 
    inline void accept(Visitor<>& v, T&) {
        throw std::domain_error(std::string(typeid(v).name()) +
            " does not implement any visitor for " + typeid(T).name());
    }
    template 
    inline void accept(Visitor<>& v, T& p) {
        if(auto* av = dynamic_cast*>(&v))
            av->visit(dynamic_cast(p));
        else
            accept(v, p);
    }
}

template 
struct Visitable : std::conditional::value, B, Visitable<>>::type {
    virtual ~Visitable() = default;
    virtual void accept(Visitor<>& v) { detail::accept(v, *this); };
    virtual void accept(Visitor<>& v) const {
        detail::accept(v, *this); };
};

template 
struct Visitable : std::conditional::value, B, detail::dummy>::type {
    virtual ~Visitable() = default;
    virtual void accept(Visitor<>&) = 0;
    virtual void accept(Visitor<>&) const = 0;
};


-
A visitor visits, it doesn't accept. If you want to ask a visitor to visit somewhere, a good name is tryVisit.

See http://coliru.stacked-crooked.com/a/603efde5268ff999 for the fully-modified version.

Code Snippets

template<class... T>
struct Visitor : Visitor<T>... {
    virtual ~Visitor() = default;
};

template<class T>
struct Visitor<T> : virtual Visitor<> {
    virtual void visit(T&) = 0;
};
template <class T>
struct Visitor<const T> : Visitor<T> {
    void visit(T& t) override { visit((const T&)t); }
    virtual void visit(const T&) = 0;
};
namespace detail {
    struct dummy {};
    template <class T>
    inline void accept(Visitor<>& v, T&) {
        throw std::domain_error(std::string(typeid(v).name()) +
            " does not implement any visitor for " + typeid(T).name());
    }
    template <class T, class X, class... Z>
    inline void accept(Visitor<>& v, T& p) {
        if(auto* av = dynamic_cast<Visitor<X>*>(&v))
            av->visit(dynamic_cast<X&>(p));
        else
            accept<T, Z...>(v, p);
    }
}

template <class B = void, class... T>
struct Visitable : std::conditional<std::is_class<B>::value, B, Visitable<>>::type {
    virtual ~Visitable() = default;
    virtual void accept(Visitor<>& v) { detail::accept<Visitable, T...>(v, *this); };
    virtual void accept(Visitor<>& v) const {
        detail::accept<const Visitable, const T...>(v, *this); };
};

template <class B>
struct Visitable<B> : std::conditional<std::is_class<B>::value, B, detail::dummy>::type {
    virtual ~Visitable() = default;
    virtual void accept(Visitor<>&) = 0;
    virtual void accept(Visitor<>&) const = 0;
};

Context

StackExchange Code Review Q#108206, answer score: 2

Revisions (0)

No revisions yet.