I'm working on a network application that implements a custom protocol. I want to easily extend the application support over the protocol as it changes. Also, as the application extends, I might need to implement other protocols. So I need a design that allows flexible packet creation, based on protocol and variety of packets that it supports.

I found that the best way to achive this is by using the Factory Pattern. (Or, for more than one protocol, an Abstract Factory).

Here is my implementation:

IPacket.h:

#pragma once
#include "PacketData.h"

class IPacket
{
public:
    // Deleted fuctions
    IPacket(const IPacket&) = delete;

    IPacket& operator=(const IPacket&) = delete;
public: 
    IPacket();
    virtual ~IPacket();

    // Encode data into a byte buffer
    virtual void Serialize(PacketData &) = 0;

    // Decode data from a byte buffer
    virtual void Deserialize(const PacketData &) = 0;
};

This provides a packet interface that will be used for further implementations of protocols.

MBXGeneric.h:

#pragma once
#include "IPacket.h"
#include "MBXHeader.h"
#include "MBXPacketFactory.h"

// Base class for custom protocol 
class MBXGenericPacket : public IPacket
{
public:
    MBXGenericPacket(MBXPacketFactory::MBXPacket _type) : H_MBX(static_cast<uint16_t>(m_Type))
    {   }

    // Serialize MBXHeader
    virtual void Serialize(PacketData& data) override
    {
        data.Data << H_MBX.h_time << H_MBX.P_Type;
    }

    // Extract MBXHeader
    virtual void Deserialize(const PacketData& data) override
    {
        data.Data >> H_MBX >> P_Type;
    }

    MBXPacketFactory::MBXPacket GetType() const { return static_cast<MBXPacketFactory::MBXPacket>( H_MBX.P_Type ); }
    
    static std::unique_ptr<IPacket> CreateGenericPacket(const MBXPacketFactory::MBXPacket Type)
    {
        return std::make_unique<MBXGenericPacket>(Type);
    }
protected:
    MBXHeader H_MBX;
};

This is the base class of an example protocol.
The packets have the following structure:

|MBX Header (Metadata + PacketType) | BODY |

The header will be initialized with each packet derrived from MBXGenericPacket. It can then be decoded or encoded in a byte buffer.

MBXPacketFactory.h:

#pragma once
#include <map>
#include <memory>
#include <mutex>
#include "MBXGeneric.h"
#include "Packets.h"

using CreateMBXPacketCb = std::unique_ptr<MBXGenericPacket>(*)();

class MBXPacketFactory
{
public:
    enum class MBXPacket : uint16_t
    {
        MBXGeneric,
        Msg_MessageBox,
        ErrorResponseMsg,

        MAX_PACKET
    };

private:
    using PacketsMap = std::map<MBXPacketFactory::MBXPacket, CreateMBXPacketCb>;
    
    PacketsMap map;
private:
    
    MBXPacketFactory()
    {
        map[MBXPacket::Msg_MessageBox] = ChatMessage::CreatePacket;
        map[MBXPacket::ErrorResponseMsg] = ErrorResponse::CreatePacket;
    }
public:
    MBXPacketFactory(const MBXPacketFactory&) = delete;
    MBXPacketFactory & operator=(const MBXPacketFactory&) = delete;
public:

    bool UnRegisterPacket(MBXPacketFactory::MBXPacket Type)
    {
        return map.erase(Type);
    }

    bool RegisterPacket(const MBXPacketFactory::MBXPacket& type, CreateMBXPacketCb callback)
    {
        return map.insert(std::make_pair(type, callback)).second;
    }

    std::unique_ptr<MBXGenericPacket> CreatePacket(MBXPacketFactory::MBXPacket Type)
    {
        auto it = map.find(Type);

        if (it == map.end())
            throw std::runtime_error("Unknown packet type!");

        return it->second();
    }

    static MBXPacketFactory& Get()
    {
        static MBXPacketFactory instance;
        return instance;
    }
};

Here's an example of packets:

// Packets.h
#include "MBXGeneric.h"
#include "MBXPacketFactory.h"

class ChatMessage : public MBXGenericPacket
{
public:
    ChatMessage() : MBXGenericPacket(MBXPacketFactory::MBXPacket::Msg_MessageBox)
    { }

    void Serialize(PacketData& data) override
    {
        data << H_MBX;

        data << sz_Title << s_Title;
        data << sz_Message << s_Message;
    }

    void Deserialize(const PacketData& data) override
    {
        // Extract the header
        data >> H_MBX;

        data >> sz_Title;
        data.read(s_Title, sz_Title);
    
        data >> sz_Message;
        data.read(s_Message, sz_Message);
    }

    static std::unique_ptr<MBXGenericPacket> CreatePacket()
    {
        return std::make_unique<ChatMessage>();
    }
public:

    // Getters and setters

    std::string GetTitle()
    {
        return s_Title;
    }

    std::string GetMessage()
    {
        return s_Message;
    }

    void SetTitle(const std::string& title)
    {
        s_Title = title;
    }

    void SetMessage(const std::string& message)
    {
        s_Message = message;
    }


private:
    int32_t sz_Title, sz_Message;

    std::string s_Title, s_Message;
};



class ErrorResponse : public MBXGenericPacket
{
public:
    ErrorResponse() : MBXGenericPacket(MBXPacketFactory::MBXPacket::ErrorResponseMsg)
    { }

    void Serialize(PacketData& data) override
    {
        data << H_MBX;
        data << sz_Error << s_ErrorMessage;
    }

    void Deserialize(const PacketData& data) override
    {
        data >> H_MBX;

        data >> sz_Error;
        data.read(s_ErrorMessage, sz_Error);
    }

    static std::unique_ptr<MBXGenericPacket> CreatePacket()
    {
        return std::make_unique<ErrorResponse>();
    }
public:

    std::string GetErrorMessage()
    {
        return s_ErrorMessage;
    }

    void SetErrorMessage(const std::string& msg_error)
    {
        s_ErrorMessage = msg_error;
    }

private:
    int32_t sz_Error;
    std::string s_ErrorMessage;
};

Example usage of factory:

When receiving a packet:

...

PacketData NewPacket = connection.RecvPacket();

uint16_t PacketID = GetUint16(NewPacket.Data);

std::unique_ptr<MBXGenericPacket> Packet = MBXPacketFactory::Get().CreatePacket(static_cast<MBXPacketFactory::MBXPacket>(PacketID));

// Up-cast the packet pointer to the right packet type

// Let's asume we've received an error response

std::unique_ptr<ErrorResponse> ErrorPacket(reinterpret_cast<ErrorResponse*>( Packet.release() ));

ErrorPacket->Deserialize(NewPacket);

std::cout << ErrorPacket->GetErrorMessage() << '\n';

...

When sending a packet:

...

std::string Title = GetUserInput();
std::string Message = GetUserInput();

std::unique_ptr<MBXGenericPacket> Interface = MBXPacketFactory::Get().CreatePacket(MBXPacketFactory::MBXPacket::Msg_MessageBox);

std::unique_ptr<ChatMessage> MessagePk(reinterpret_cast<ChatMessage*>( Interface.release() ));

MessagePk->SetTitle(Title);
MessagePk->SetMessage(Message);

PacketData MsgPacketData;
MessagePk->Serialize(MsgPacketData);

PacketQueue.push(MsgPacketData);

...

Improvements:

What I would like to improve in the current design:

• The up-casting from the base class to the derived class in order to access member functions.
• With each new packet derived from MBXGenericPacket, I'm forced to encode and decode the MBX_Header in the serialization/deserialization, thus causing code repetition.
• To get the required packet class from the factory, I need to first extract the identifier from the header and then pass it to the CreatePacket() method. I think it will be nice if you could just pass the PacketData to the factory, rather then decoding information by yourself.
• The factory's purpose is to abstract the creation process of classes. In my opinion, the packet identifiers should be encapsulated by the factory, but that causes a lot of noise in the code, as you have to type: MBXPacketFactory::MBXPacket:: ...

Let me know your thoughts about the code. I'm here to learn, so any opinion is welcomed !