The problem is your Visitor
class is tightly coupled with classes that derive from Element
. As you expand your design this is going to get in the way more than it already is. You can reduce/eliminate the right coupling by providing a "destination" class that defines all the requirements of a visitable object. Since the name of a derived classes is a common attribute you can place the storage and access to it into the destination class as well.
// 1. Define out visitor and destination interfaces
struct Destination
{
Destination(const std::string& name) : name_(name) {}
virtual std::string ident() const { return name_; }
const std::string name_;
};
struct Visitor
{
virtual void visit(Destination *e) = 0;
};
This keeps the requirements of the visitor separate from the Element class which seems to be your intention. Then your This
and That
classes inherit from Destination
and provide the necessary implementations.
// 2. Define our element and it's derived classes
class Element
{
public:
virtual void accept(class Visitor &v) = 0;
};
template <unsigned int N>
class This: public Element, public Destination
{
public:
This() : Destination("This") {}
virtual void accept(Visitor &v)
{
v.visit(this);
}
};
class That: public Element, public Destination
{
public:
That() : Destination("That") {}
virtual void accept(Visitor &v)
{
v.visit(this);
}
};
Now your up and down visitors are simplified into something like the following
// 3. Create a "visitor" derived class for each "operation" to do on "elements"
class UpVisitor: public Visitor
{
void visit(Destination *e) {
cout << "do Up on " + e->ident() << '\n';
}
};
class DownVisitor: public Visitor
{
void visit(Destination *e) {
cout << "do Down on " + e->ident() << '\n';
}
};
Although I did not change it in the solution above I recommend changing visit
to take a reference instead of a pointer. Since C++ has no notion of a null reference this indicates that Destination
is required where as a pointer could be considered optional.