Visitor Pattern and std::shared

I am using the Visitor design pattern in my application for message processing. For example:

class AbstractMessageVisitor;

class AbstractMessage {
public:
      virtual void accept(AbstractMessageVisitor& visitor) const = 0;
};

class A : public AbstractMessage {
public:
    void accept(AbstractMessageVisitor& visitor) const override { visitor.visit(*this); }
};
class B : public AbstractMessage { 
    //... 
};

class AbstractMessageVisitor {
    friend class A;
    friend class B;

protected:
    virtual void visit(const A&) {};
    virtual void visit(const B&) {};
};

In my application, instances of the messages are being created as std::shared_ptr. If I "receive" one of these messages created by the factory, I would "visit" it by doing the following:

class MessageHandler : public AbstractMessageVisitor {
public:
    void handleMessage(std::shared_ptr<const AbstractMessage> msg) {
         msg->accept(*this);
    }

protected:
    void visit(const A& msg);
    void visit(const B& msg);
};

In this case, I realized that my visit methods may need to "save the message for later." Since I know that the message that is being visited is managed by a std::shared_ptr, I figured I could just copy the shared_ptr to use later.

However, this is where I encountered my problem; within the visit methods there is no shared_ptr.

Here are some of the solutions I found to the problem:

Option 1

Add a member variable to MessageHandler class that temporarily stores the shared_ptr while the message is being visited. If the visit method needs the pointer, it makes a copy of this member variable.

class MessageHandler : public AbstractMessageVisitor {
public:
    void handleMessage(std::shared_ptr<const AbstractMessage> msg) {
         _ptr = msg;
         msg->accept(*this);
         _ptr.reset();
    }

protected:
    void visit(const A& msg) {
        auto thePointer = std::static_pointer_cast<const A>(_ptr);
    }
    void visit(const B& msg);

private:
    std::shared_ptr<AbstractMessage> _ptr;
};

Obviously, this has many problems with it. You have to cast the temporary shared pointer into the appropriate type. This kind of defeats the purpose of the visitor pattern. You have this "shared state" that must be kept as a member variable.

Option 2

Inherit from std::enable_shared_from_this.

class A : public AbstractMessage, std::enable_shared_from_this<A> {
public:
    inline auto ptr() const { return shared_from_this(); }
    //...
};

However, this only works if you can be guaranteed that the class is owned by a shared_ptr. On GCC, if it is not, it looks like an exception will be thrown or, if exceptions are disabled, the program will immediately exit.

Looking at the implementation of enable_shared_from_this, I wonder why shared_from_this could not just return a nullptr if the object is NOT owned by a shared_ptr, but alas...

Option 3

Just make the object "cloneable".

class A : public AbstractMessage {
public:
    std::shared_ptr cloneShared() { return std::make_shared<A>(*this); }
    //...
};

This isn't really a solution, as it is not taking advantage of the fact that a shared_ptr to the object already exists.

So, my questions are:

Is there a way to accomplish what I am trying to do?
Perhaps this problem is coming from a flaw in my design; is there something I should change?

CodePudding user response：

I believe I found an answer. It is based upon the following two references:

https://en.cppreference.com/w/cpp/memory/enable_shared_from_this/weak_from_this

Check whether an object derived from enable_shared_from_this is managed by shared_ptr?

It appears that, if you use the weak_from_this() method instead of shared_from_this(), an exception will NOT be generated. Instead the behavior is as I would expect, if the object is NOT owned by a shared_ptr, the weak_ptr returned will be invalid (i.e., .lock() will return a nullptr).

Thus, I can do the following in my example:

class A : public AbstractMessage, std::enable_shared_from_this<A> {
public:
    inline auto ptr() const noexcept { return weak_from_this().lock(); }
    void accept(AbstractMessageVisitor& visitor) const override { visitor.visit(*this); }
};

class MessageHandler : public AbstractMessageVisitor {
public:
    void handleMessage(std::shared_ptr<const AbstractMessage> msg) {
         msg->accept(*this);
    }

protected:
    void visit(const A& msg) {
        _savedForLater = msg.ptr();
    }
    void visit(const B& msg);

private:
    std::shared_ptr<A> _savedForLater;
};

CodePudding user response：

THis

class AbstractMessageVisitor {
    friend class A;
    friend class B;

protected:
    virtual void visit(const A&) {};
    virtual void visit(const B&) {};
};

Is very strange. Your Visitor has to be told expressly about each message type.

Surely it should be

class AbstractMessageVisitor {


protected:
    virtual void visit(const AbstractMessage&) {};
};

Ie the visitor deals with instances of Abstract message derived classes. I would have

using AbstractMessagePtr = std::shared_ptr<AbstractMessage>;

...
   virtual void visit(AbstractMessagePtr msg) {};

ie always be dealing with shared_ptr to messages

If you need to keep the pointer for a while just have a member vaiable of type AbstractMessagePtr in the class