0

I have a tree of polymorphic objects. I need to traverse the two trees and compare the nodes. If the nodes have different types, they are not equal. Consider this hierarchy:

struct Visitor;

struct Base {
  virtual ~Base() = default;
  virtual void accept(Visitor &) = 0;
};
using BasePtr = std::unique_ptr<Base>;

struct A final : Base {
  void accept(Visitor &) override;
  int data;
};
struct B final : Base {
  void accept(Visitor &) override;
  BasePtr child;
};
struct C final : Base {
  void accept(Visitor &) override;
  std::vector<BasePtr> children;
};

struct Visitor {
  virtual void visit(const A &) = 0;
  virtual void visit(const B &) = 0;
  virtual void visit(const C &) = 0;
};

I know how to implement these functions:

bool equalNode(const A &, const A &);
bool equalNode(const B &, const B &);
bool equalNode(const C &, const C &);

I'm asking about how I should implement this function:

bool equalTree(const Base *, const Base *);

How do I elegantly go from equalTree to equalNode possibly using the visitor pattern?

Indiana Kernick
  • 5,041
  • 2
  • 20
  • 50
  • Note: `Base` should have `virtual ~Base()`. – Evg Sep 05 '18 at 10:37
  • purpose of visitor class is to perform action with class which called its overloaded method (in your case `visit`) with `*this` as an argument. So your general description of Visitor is useless, because you need two instances of class. A possible solution might be a visitor Compare, which would take reference to a second class as an argument on construction, though I not sure if visitor pattern is beneficial here. Another variant is to design a custom accept\visitor dispatcher in base class, which would take `Base*` as an argument and perform comparison. – Swift - Friday Pie Sep 05 '18 at 10:56
  • it could be an `operator==` :P – Swift - Friday Pie Sep 05 '18 at 11:02
  • "If the nodes have different types, they are not equal." This violates LSP and so isn't really OO-style polymorphism. – n. m. could be an AI Sep 05 '18 at 11:10
  • @n.m. how? the operation is "are these trees equal". It's entirely expected that different values are unequal – Caleth Sep 05 '18 at 11:40
  • @Caleth Say "triangle" and "coloured triangle" are different types, but LSP suggests that two such triangles with equal vertices should be equal. – n. m. could be an AI Sep 05 '18 at 11:43
  • @n.m. no it doesn't. And even if it did, the *value* of an `A` object is always unequal to the *value* of a `B` or `C` object *in this case* – Caleth Sep 05 '18 at 11:46
  • @n.m. you might define an equivalence relation for "triangle" and "coloured triangle", where the base is not abstract, but this case is more like "triangle", "rectangle", "circle". *by definition* the vertices are unequal collections – Caleth Sep 05 '18 at 11:48
  • @Caleth "no it doesn't" I cannot agree with this. If you can guarantee that an object of a concrete class is never compared with an object of its subclass, then it's possible define comparison that meets the requirement and is LSP compliant. Otherwise, no. – n. m. could be an AI Sep 05 '18 at 12:03
  • @n.m. We aren't discussing subclasses of concrete classes. All the concrete classes here are `final` – Caleth Sep 05 '18 at 12:54
  • @Caleth OK so in this special case it is possible. – n. m. could be an AI Sep 05 '18 at 13:25

2 Answers2

1

Something like

struct RhsVisitor : public Visitor
{
    bool result;
};

struct AEqualVisitor : public RhsVisitor
{
    void visit(const A & rhs) override { result = equalNode(lhs, rhs); }
    void visit(const B &) override { result = false; }
    void visit(const C &) override { result = false; }

    const A & lhs;
};

And similar for B and C

struct LhsVisitor : public Visitor
{
    void visit(const A & a) override { rhsVisitor = std::make_unique<AEqualVisitor>(a); }
    void visit(const B & b) override { rhsVisitor = std::make_unique<BEqualVisitor>(b); }
    void visit(const C & c) override { rhsVisitor = std::make_unique<CEqualVisitor>(c); }

    std::unique_ptr<RhsVisitor> rhsVisitor;
};

bool equalTree(const Base * lhs, const Base * rhs)
{
    LhsVisitor vis;
    lhs->accept(vis);
    rhs->accept(*vis.rhsVisitor);
    return vis.rhsVisitor->result;
};
Caleth
  • 52,200
  • 2
  • 44
  • 75
  • Interesting. I wonder if `AEqualVisitor` can become `EqualVisitor` somehow – Indiana Kernick Sep 05 '18 at 11:12
  • @Kerndog73 on second thought, not easily. I don't think you can have a template member override a base class's virtual member – Caleth Sep 05 '18 at 11:21
  • Hmm... I think I'll have a play around with this – Indiana Kernick Sep 05 '18 at 11:22
  • If you made the `accept`s templates, you could drop the `Visitor` class, and then it would work fine to just call `vistor.visit(*this);`. You swap an interface for a concept – Caleth Sep 05 '18 at 11:26
  • I'm not quite sure what you mean. At some point you would need to do a virtual call or a dynamic cast. – Indiana Kernick Sep 05 '18 at 11:29
  • Nope, you just assume that the type deduced for `template ` has a member `visit` that can be called with an `A`, `B` or `C`. see [`std::visit`](https://en.cppreference.com/w/cpp/utility/variant/visit) (which uses `operator()` as the member to call) – Caleth Sep 05 '18 at 11:31
  • Could you update your answer with an example? It almost sounds like you're getting the runtime type at compile-time. – Indiana Kernick Sep 05 '18 at 11:34
  • I'm not sure it's worth it. You still need `RhsVisitor` to declare all the required `visit`s, because you need the runtime polymorphism in `LhsVisitor` – Caleth Sep 05 '18 at 11:37
  • I ended up using an approach very similar to this. I used LhsVisitor to get the type of the left node. Then I passed the type as a template parameter to RhsVisitor which gets the type of the right node. The visit method of RhsVisitor calls equalNode. There is a templated overload of equalNode which returns false if the given types are different. I think I see what you mean by templating the accept methods because the bodies of the visit methods for each type are identical. I used macros to handle to duplication but I think there’s a way to use templates instead. – Indiana Kernick Sep 05 '18 at 12:32
  • I’ll except this answer (because it’s the solution I’m using) in 24 hours (to give others a chance to suggest a better solution). I know that saying “thank you” is frowned upon here but I have to say it! Thank you @Caleth, this discussion was very helpful – Indiana Kernick Sep 05 '18 at 12:36
0

There are three approaches here.


The first is - you need to do double dispatch. You have one visitor for one side and one visitor for the other side. Once you pick one side, you "save" that type as a template parameter, which you can use to visit the other side:

template <typename T>
struct Right : Visitor {
     Right(T const* lhs) : lhs(lhs) { }
     T const* lhs;
     bool result;

     void visit(A const& x) override { visit_impl(x); }
     void visit(B const& x) override { visit_impl(x); }
     void visit(C const& x) override { visit_impl(x); }

     void visit_impl(T const& x) { result = equalNode(*lhs, x); }
     template <typename U> void visit_impl(U const&) { result = false; }
};

struct Left : Visitor {
    Left(Base const* lhs, Base const* rhs) : rhs(rhs) {
        lhs->accept(*this);
    }
    Base const* rhs;
    bool result;

    void visit(A const& x) override { visit_impl(x); }
    void visit(B const& x) override { visit_impl(x); }
    void visit(C const& x) override { visit_impl(x); }

    template <typename U>
    void visit_impl(U const& lhs) {
        Right<U> right(&lhs);
        rhs->accept(right);
        result = right.result;
    }
};

bool equalTree(const Base *lhs, const Base *rhs) {
    return Left(lhs, rhs).result;
}

The second is - you cheat. You only care about the cases where the two sides are the same type, so you only need single dispatch:

struct Eq : Vistor {
    Eq(Base const* lhs, Base const* rhs) : rhs(rhs) {
        lhs->accept(*this);
    }

    Base const* rhs;
    bool result;

    void visit(A const& x) override { visit_impl(x); }
    void visit(B const& x) override { visit_impl(x); }
    void visit(C const& x) override { visit_impl(x); }

    template <typename U>
    void visit_impl(U const& x) {
        result = equalNode(x, *static_cast<U const*>(rhs));
    }
};

bool equalTree(Base const* lhs, Base const* rhs) {
    if (typeid(*lhs) == typeid(*rhs)) {
        return Eq(lhs, rhs).result;
    } else {
        return false;
    }
}

The third is - you don't do this through OO and instead use a variant:

using Element = std::variant<A, B, C>;

struct Eq {
    template <typename T>
    bool operator()(T const& lhs, T const& rhs) const {
        return equalNode(lhs, rhs);
    }

    template <typename T, typename U>
    bool operator()(T const&, U const&) const {
        return false;
    }
};

bool equalTree(Element const& lhs, Element const& rhs) {
    return std::visit(Eq{}, lhs, rhs);
}
Barry
  • 286,269
  • 29
  • 621
  • 977
  • I don't think the second works, surely`typeid(*lhs)` is `typeid(Base)`? – Caleth Sep 05 '18 at 15:41
  • @Caleth No, it's not. `typeid` gives run-time type information. So while `typeid(lhs)` would give you the same thing as `typeid(Base const*)`, `typeid(*lhs)` would give you `typeid(A)` or something. – Barry Sep 05 '18 at 17:29
  • Your first approach is what I ended up doing. Although your second approach looks a lot cleaner. Is the second approach faster than the first? Is comparing stings faster than 4 virtual calls? Your third approach can’t be used in my situation as I am already using the polymorphic tree in a lot of other code. – Indiana Kernick Sep 06 '18 at 03:42