Decoupling receiving data and processing data by passing callback and shared_ptr

Question

Sorry for my poor English. I want to decouple receiving data and processing data, this is the demo code:

#include<memory>
#include<iostream>
#include<functional>

using message_handler=std::function<void(const std::string&)>;

class Session: public std::enable_shared_from_this<Session>
{
class PacketProc
{
public:
    void SetOnMsgProc(const message_handler &&on_msg_func)
    {
        this->inner_on_msg_func = on_msg_func;
    }

    void ProcRcved(const std::string &str)
    {
        if(inner_on_msg_func)
        {
            inner_on_msg_func(str);
        }
    }

private:
    message_handler inner_on_msg_func; 
};

public:
    ~Session()
    {
        std::cout << "~Session()" << std::endl;
    }

    void Init(void)
    {
        std::weak_ptr<Session> weak=shared_from_this();
        packet_proc.SetOnMsgProc([weak](const std::string & str){
            auto shared = weak.lock();
            if(shared && shared->outter_on_msg_func)
            {
                shared->outter_on_msg_func(str);
            }
        });

        outter_on_msg_func= [](const std::string & str)
        {
            std::cout << "recieved:" << str << std::endl;
        };
    }

    void Proc(void)
    {
        std::string str = read();
        packet_proc.ProcRcved(str);
    }

    std::string read(void)
    {
        return std::string("this is a test");
    }

private:
    PacketProc packet_proc;
    message_handler outter_on_msg_func;
};

int main()
{
    std::shared_ptr<Session> pSession= std::make_shared<Session>();
    pSession->Init();
    pSession->Proc();
}

Any suggestion and advice is welcome.

Answer 1

The declaration for the class PacketProc should be indented, right now the code is hard to read.

class Session : public std::enable_shared_from_this<Session>
{
    class PacketProc
    {
    public:
        void SetOnMsgProc(const message_handler&& on_msg_func)
        {
            this->inner_on_msg_func = on_msg_func;
        }

        void ProcRcved(const std::string& str)
        {
            if (inner_on_msg_func)
            {
                inner_on_msg_func(str);
            }
        }

    private:
        message_handler inner_on_msg_func;
    };

public:
    ~Session()
    {
        std::cout << "~Session()" << std::endl;
    }

    ...
};

If the class PacketProc is supposed to be reusable then it should be declared outside the class Session, right now it can only be accessed through the Session class.

To follow best practices, operators such as = should have blank spaces on both sides:

using message_handler = std::function<void(const std::string&)>;

That would also improve the readability of the code.

Using the this pointer really shouldn't be necessary in this code:

                void SetOnMsgProc(const message_handler&& on_msg_func)
                {
                    inner_on_msg_func = on_msg_func;
                }

Inside a class it generally isn't necessary to use the this pointer, it is the default. The only time this isn't true is is if an identifier has 2 possible uses within the same scope of the code, such as if inner_on_msg_func was defined as a global in the Session class and a local in the PacketProc class.