1

Currently I'm writing the message handling system for my game server. I'm using double-dispatch design pattern to pick the needed handler according to message id, which retrieved from a data buffer. But I ran into a following problem: I need to determine the derived class of the message to pass it into a double-dispatch handler object. the only solution I found is to write a factory method in base class with the switch inside, that will create needed derived class according to message id.

class Message
{
public:
    void dispatch(Handler& handler)
    {
        dispatchImpl(handler);
    }

protected:
    virtual void dispatchImpl(Handler& handler) = 0;
};

template <typename TDerived>
class BaseMessage : public Message 
{
public:
    BaseMessage(unsigned short messageId)
        : id(messageId) {};

    virtual ~BaseMessage() = default;

    unsigned short getId() const { return id; }

    static std::unique_ptr<Message> create(BitStream& data)
    {
        switch (data.ReadShort())
        {
            case 1: return std::make_unique<FirstMessage>();
            case 2: return std::make_unique<SecondMessage>();
            case 3: return std::make_unique<ThirdMessage>();
            // ...
        }
    }

protected:
    unsigned short id;

    virtual void dispatchImpl(Handler& handler) override
    {
        handler.handle(static_cast<TDerived&>(*this));
    }
};

How can I improve my design to avoid big-switch statement? Thanks in advance.

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
Alex Kuzub
  • 13
  • 2

1 Answers1

3

You might use a std::map<short,std::function<std::unique_ptr<Message> (BitStream& data)> and allow registration of those factory functions dynamically:

std::map<short,std::function<std::unique_ptr<Message> (BitStream& data)> createMessageHandlers;

void registerCreateMessageHandler(short id,std::function<std::unique_ptr<Message> (BitStream& data)> fn) {
      createMessageHandlers[id] = fn;
}

and use it like

registerCreateMessageHandler(1, [](BitStream& data) { return std::make_unique<FirstMessage>();});
registerCreateMessageHandler(2, [](BitStream& data) { return std::make_unique<SecondMessage>();});

Your message type constructors should probably take the BitStream& as a constructor parameter.

You should consider to use a tool like google protocol buffers to define your communication protocols.
That would help a lot in properly versioning, and parsing message packets (endianess neutral) over the wire (or wireless).

In combination with e.g. boost::asio or zeromq for the transport handling, that should give you the maximum flexibility and proper mechanisms to separate the transport and semantical layers of communication.

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
  • Thanks for your feedback. I'm developing a high-load game server. Tools like protobuf will cause perfomance drops. – Alex Kuzub Jan 07 '19 at 19:06
  • @AlexKuzub I never really had observed performance issues with _google protocol buffers_. Just the opposite, the transport protocol is optimized as can be, regarding savior of bandwith and parsing. Performance drops might be more because unnecessary roundtrips used in the overall design. You should review these again probably. – πάντα ῥεῖ Jan 07 '19 at 19:09
  • Thanks. And what you can say about the overall design? Is it suitable for this system and properly implemented? Would it be a better solution to use a function pointers map, where I will both deserialize and handle a messages? – Alex Kuzub Jan 08 '19 at 09:55
  • @AlexKuzub Whatever message format you utilize, a map and function pointer (callables) handlers would be the most flexible design. As mentioned google protobuf already adds up the stuff necessary for de-/serialization. – πάντα ῥεῖ Jan 08 '19 at 09:57