I'm writing a typical client-server architecture for a chat program. My goal is to make the protocol flexible so that I could add more functionality in the future. (Any good software should allow that)
Both applications should be able to wait for a response from a particular request that has been sent.
I'm currently using two main design patterns in my code:
- Command Pattern - Requests and responses will be encapsulated as objects
- Visitor Pattern - To process commands
Command pattern:
Each command inherits from the ChatPacket base class:
// chat_packet.hpp
#include "Processor/IProcessor.hpp"
class ChatPacket
{
public:
ChatPacket() = default;
ChatPacket(uint8_t type)
: pk_type(type)
{}
template <class Archive>
void serialize(Archive& arc)
{ arc(pk_type); }
virtual void process(IProcessor& proc) = 0;
virtual ~ChatPacket() {}
protected:
enum PacketType : uint8_t
{
Chat_Control,
Chat_Error,
Chat_Message,
MAX_PKTYPE
};
protected:
uint8_t pk_type;
};
I've divide commands in categories for easy management. Each base category could be divided into more subcategories and so on, thus creating hierarchies of commands, as you can see in the following example:
// chat_control.hpp
#include "chat_packet.hpp"
class ChatControl : public ChatPacket
{
public:
ChatControl(uint8_t control_msg)
: ChatPacket(Chat_ControlMsg),
ctrl_type(control_msg)
{}
ChatControl() : ChatPacket(Chat_ControlMsg)
{}
virtual void process(IProcessor& proc){
proc.process(*this);
}
template <class Archive>
void serialize(Archive& arc)
{ arc(ctrl_type);}
protected:
enum CtrlMessages : uint8_t
{
Ctrl_Kick,
Ctrl_MakeAdmin,
Ctrl_CloseChatRoom,
Ctrl_ListUsers,
MAX_CTRL_MSG
};
protected:
uint8_t ctrl_type;
};
class KickUser : public ChatControl
{
public:
KickUser(const std::string& user)
: ChatControl(Ctrl_Kick), user_to_kick(user)
{}
KickUser() : ChatControl(Ctrl_Kick)
{}
template <class Archive>
void serialize(Archive& arc)
{ arc(user_to_kick); }
void process(IProcessor& proc) override{
proc.process(*this);
}
public:
std::string user_to_kick;
};
class CloseChatRoom : public ChatControl
{
public:
CloseChatRoom(const std::string& user)
: ChatControl(Ctrl_CloseChatRoom), username(user)
{}
CloseChatRoom() : ChatControl(Ctrl_CloseChatRoom)
{}
template <class Archive>
void serialize(Archive& arc)
{ arc(username);}
void process(IProcessor& proc) override{
proc.process(*this);
}
public:
std::string username;
};
class MakeChatAdmin : public ChatControl
{
public:
MakeChatAdmin(const std::string& username)
: ChatControl(Ctrl_MakeAdmin), user_to_admin(username)
{}
MakeChatAdmin() : ChatControl(Ctrl_Make_Admin)
{}
template <class Archive>
void serialize(Archive& arc)
{ arc(user_to_admin);}
void process(IProcessor& proc) override{
proc.process(*this);
}
public:
std::string user_to_admin;
};
class ListChatParticipants : public ChatControl
{
public:
ListChatParticipants(const std::vector<std::string>& list)
: ChatControl(Ctrl_ListUsers), user_list(list)
{}
ListChatParticipants() : ChatControl(Ctrl_ListUsers)
{}
template<class Archive>
void serialize(Archive& arc)
{ arc(user_list); }
void process(IProcessor& proc) override{
proc.process(*this);
}
public:
std::vector<std::string> user_list;
};
// chat_message.hpp
#include "chat_packet.hpp"
class ChatMessage : public ChatPacket
{
public:
ChatMessage(const std::string& msg)
: ChatPacket(Chat_Message), message_body(msg)
{}
ChatMessage() : ChatPacket(Chat_Message)
{}
template <class Archive>
void serialize(Archive& arc)
{ arc(message_body); }
void process(IProcessor& proc){
proc.process(*this);
}
public:
std::string message_body;
};
The current approach has a couple of downsides:
- There's an explosion of classes, implicitly lots of functions to overload to support each type when processing is required. (A major problem when you try to implement the good old Visitor pattern)
- Code repetition: the structure of classes is pretty much the same (You would expect this, as I use inheritance to define my commands)
Visitor Pattern
When having such a rich collection of commands (one class for each command), the visitor pattern makes you question your life. It's tricky to implement and has more downsides than advantages, but it's still widely used.
The implementation is the basic one. First the abstract visitor type:
// IProcessor.hpp
#pragma once
class ChatMessage;
class ChatControl;
class KickUser;
class CloseChatRoom;
class MakeChatAdmin;
class ListChatParticipants;
class IProcessor
{
public:
virtual void process(ChatMessage&) = 0;
virtual void process(ChatControl&) = 0;
virtual void process(KickUser&) = 0;
virtual void process(CloseChatRoom&) = 0;
virtual void process(MakeChatAdmin&) = 0;
virtual void process(ListChatParticipants&) = 0;
};
Followed by concrete implementations of processors(or handlers):
//chat_control_processor.hpp
#pragma once
#include "IProcessor.hpp"
#include "chat_room.h"
class ChControlHandler : public IProcessor
{
public:
ChControlHandler(ChatRoom& ch_room)
: Room(&ch_room)
{}
public:
// This function does nothing, as
// The type is not suported by the current handler
void process(ChatMessage&) override;
void process(ChatControl&) override;
void process(KickUser&) override;
void process(CloseChatRoom&) override;
void process(MakeChatAdmin&) override;
void process(ListChatParticipants&) override;
private:
std::unique_ptr<ChatRoom> Room;
};
Now you should see the main problem with the current design, and if you don't... check your glasses.
I'm using cereal library for serialization which automatically constructs objects from pointers to base class, which makes enums in base classes pretty much useless. I kept them just for logging purposes.
I'm looking for an alternative to the visitor pattern that will allow one handler per commands category and overall advice on the current design and program structure. Any suggestions on naming convensions, best practices and opinions on the code are welcomed!
