0

I am using a Visitor pattern to traverse and print the children of the tree I am operating on. To get indented printing, I specify the indentation level in a style like:

printCurrent();
indentLevel();        // increases static variable
Visitor::visit(elem); // which then prints the children's node data
unindentLevel();      // decreases static variable

In order to make this nicer, I want to implement a function that takes the Visitor::visit with the argument elem and automatically handles the pre-action (indentLevel()) and post-action (unindentLevel()).

Before implementing that function, I need to define a function pointer that will be used as argument for the function. However, I am failing at specifying the argument to the pointer. As an example, let's look into the PrintVisitor which is derived from the Visitor:

void PrintVisitor::visit(BinaryExpr &elem) {
    std::cout << formatOutputStr({elem.getNodeName()});
    this->incrementLevel();
    Visitor::visit(elem);    // <-- this is where I want to create a function pointer to
    this->decrementLevel();
}

The idea basically is that the PrintVisitor does everything related to printing and all other logic (e.g., traversal logic) is implemented in the base class Visitor. Hence the PrintVisitor::visit needs to execute its specific action (e.g., printing via formatOutputStr) and then execute the Visitor::visit method:

void PrintVisitor::visit(BinaryExpr &elem) {
    std::cout << formatOutputStr({elem.getNodeName()});

    void (Visitor::*myPt)(BinaryExpr&) = &Visitor::visit;  // declare function pointer
    executeIndented(myPt, elem);  // pass function pointer myPt
}
// ...

void executeIndented("Function f", "FunctionArgs elem") {
   // pre-action
    this->incrementLevel();

   // main action: call function pointer
   (Visitor().*f)(elem);  // call function pointer with arg

   //post-action
   this->decrementLevel();    
}

My goal is to somehow achieve that both pre- and post-action always are called in each PrintVisitor::visit method. For that I was thinking it would make sense to encapsulate these pre- and post-actions into another function executeIndented which ensures that.

The syntax of (Visitor().*myPt)(elem); looks a little odd to me, is this really the correct way to call the (base) function Visitor::visit with the argument elem using my function pointer myPt?

// EDIT Using (Visitor(*this).*myPt)(elem); instead also works. What's the difference between those two ways and is either one of those to be preferred?

// EDIT2 Hope that the description of what I am trying to achieve is more clear now.

Patrick
  • 1,046
  • 2
  • 10
  • 31
  • Does this answer your question? [C++ Call Pointer To Member Function](https://stackoverflow.com/questions/14814158/c-call-pointer-to-member-function) – UnholySheep Dec 17 '19 at 10:41
  • @UnholySheep No, unfortunately it doesn't as it doesn't consider inheritance. – Patrick Dec 17 '19 at 10:44
  • 2
    `Visitor(*this)` does object-slicing copy. `Visitor()` use default constructor... – Jarod42 Dec 17 '19 at 10:44
  • @Jarod42 So in case that a virtual function of the base class is to be called it does not matter, right? – Patrick Dec 17 '19 at 10:47
  • I suppose the base Class Visitor does nothing (but traversal), so both version would result in nothing... – Jarod42 Dec 17 '19 at 10:58
  • 1
    @Patrick, It matters because `Visitor(*this)` is copy constructor call and `Visitor()` is default constructor as mentioned above, so both objects may have different state (depending on there constructors), also those two calls creates different objects of type `Visitor` from `PrintVisitor` is that what you want? or you just want to call base class `visit` function from `PrintVisitor`. I am not sure that i understand your original question correctly. – Gaurav Dhiman Dec 17 '19 at 11:05
  • @GauravDhiman I see! Thanks for the explanation. Yes, exactly! I just wanted to call the `visit` method from the current element's (`elem`) base class `Visitor`. I suppose that should be possible somehow without creating a new `Visitor` object. – Patrick Dec 17 '19 at 11:07
  • 1
    @Patrick whats wrong with calling `Visitor::visit(elem);?` from `PrintVisitor` since `PrintVisitor` is derived from `Visitor` – Gaurav Dhiman Dec 17 '19 at 11:12
  • You can even call a base method from code outside the class, like this: `PrintVisitor pv; pv.Visitor::visit(elem);` This will call the base method directly (even if the function is declared virtual). – Balázs Kovacsics Dec 17 '19 at 11:17
  • @GauravDhiman I want to be able to pass the function call `Visitor::visit` to a method which automatically performs some things before and afterwards. Or is there any other way to implement a construct to execute sth. before and after? – Patrick Dec 17 '19 at 11:19
  • 1
    FYI: (my answer to) [SO: Preorder traversal through Morse code BST](https://stackoverflow.com/a/45055152/7478597) with focus on `BSTreeT::traverse()`. – Scheff's Cat Dec 17 '19 at 11:21
  • Why do you want to pass through a member function pointer. In other words, why `Visitor::visit(elem);` is not enough? – AProgrammer Dec 17 '19 at 12:04
  • @AProgrammer Please see my edited description to see what I am trying to achieve. – Patrick Dec 17 '19 at 12:50

1 Answers1

1

As I understand, you should have something like:

struct TraversalVisitor : IVisitor
{
    void visit(BinaryExpr &elem) final
    {
        pre_traversal_action(elem);
        visit(elem.lhs);
        action(elem);
        visit(elem.rhs);
        post_traversal_action(elem);
    }
    virtual void pre_traversal_action(BinaryExpr &elem) { /*Nothing */ }
    virtual void action(BinaryExpr &elem) { /*Nothing */ }
    virtual void post_traversal_action(BinaryExpr &elem) { /*Nothing */ }

    void visit(UnaryExpr &elem) final;
    // ...
};

struct PrintVisitor : TraversalVisitor
{
    void pre_traversal_action(BinaryExpr &elem) override { 
        std::cout << formatOutputStr({elem.getNodeName()});
        incrementLevel();
    }
    //void action(BinaryExpr &elem) override { /*Nothing */ }
    void post_traversal_action(BinaryExpr &elem) override { decrementLevel(); }
    // ...
private:
    void formatOutputStr(const std::string&);
    void incrementLevel();
    void decrementLevel();
    // ...
};

Whereas you try to implement something like:

struct Visitor : IVisitor
{
    virtual visit(BinaryExpr &elem)
    {
        visit(elem.lhs);
        visit(elem.rhs);
    }
// ...
};

struct PrintVisitor : Visitor
{
private:
    void formatOutputStr(const std::string&);
    void incrementLevel();
    void decrementLevel();

    void executeIndented(Expr& elem) {
        incrementLevel(); // pre-action

        // Traversal
        Visitor::visit(elem);

        decrementLevel(); // post-action
    }

    void visit(BinaryExpr &elem) override {
        std::cout << formatOutputStr({elem.getNodeName()});

        executeIndented(elem);
    }

    // ...
};

Your attempt, IMO, just factorize PrintVisitor, without enforcing some traversal strategy.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • That's very close to what I intented and can be used instead, thank you! My original idea though was to perform the `action` right in the `visit` method: Instead of having a separate `pre_traversal_action` and `post_traversal_action`, to have a single method `pre_post_traversal(Function f)` which takes a function `f` (e.g. `Visitor::visit`) with its args (`elem`) and then (1) applies the pre-traversal action, (2) runs the function `f`, and runs the (3) post-traversal action. The goal of this is to enforce the post-traversal action instead of requiring to specify it in each `visit(...)` method. – Patrick Dec 17 '19 at 12:37
  • My current version requires pre/post action by *node*, but can easily be twitted to be shared by any node: `virtual void post_traversal_action(BinaryExpr &)` -> `virtual void post_traversal_action(IExpr&)`. – Jarod42 Dec 17 '19 at 15:06
  • Added a version which should be more what you try to implement. – Jarod42 Dec 17 '19 at 15:06
  • You're a genius! That's exactly what I was trying to achieve—much more complicated though. Thanks a lot for your patience & help. I need to admit that your first solution seems to be more reasonable and much more generic. I think I will try to combine both solutions. – Patrick Dec 17 '19 at 15:44