Re: design/refactoring of message-based system in C++: ideal solution?
On Nov 24, 6:27 am, SegFault <altctrlsys...@gmail.com> wrote:
The problem:
I can't think of a good(perfect) design for new message system - they
all seem to have flaw at one point or another. It isn't necessary to
preserve existing protocol, but I need something more flexible.
Currently system uses switch/case to process different messages, which
is ugly, and hard to maintain. Also, currently system doesn't have
structs for every message type. I'm currently trying to create struct/
class for every message type, with common base class that has virtual
method "Save to packet", then create std::map which will map message
types to message constructors (i.e. factory pattern), then maybe turn
"messages" into "command" pattern. However, it is still far from
perfect - I can't initialize map with constructors automatically, for
example, I'll have to create quite a lot of redundant structs with
lots of virtual methods, etc. Is there a better way to do this?
I'd call your problem a serialization (a.k.a persistence) one ;-).
Certainly a good starting point is to define a type (class/struct) for
every actual message, and to have save and load methods in there.
Now... You have to read at least something before deciding the type of
the message, I'd say that is common for any "polymorphic"
serialization scheme (your first byte, perhaps more). That's OK from
where I stand, and that gives e.g.:
struct base_message_type
{
virtual void load(load_stream&) = 0;
virtual void store(store_stream&) = 0;
};
Mapping message_type<=>creation_facility using std::map is fine, too.
For creation_facility, simplest possible is to reach for a function
pointer. But you will probably need more, so you'll end up with some
sort of a meta-class (description of a message_type). That would be a
dreaded (one per class) singleton, e.g.
struct message_type_class
{
virtual base_message_type* create_object(char wire_message_type) =
0;
// Other things...
}
Description of the system:
1) Approximately 2 megabytes of code.
2) Uses approximately 70 types of messages, message type is indicated
by "uchar", which is sent first.
3) Each message type has different number of parameters (typically
longs and ints, sometimes strings).
4) Each message type has fixed number of parameters, different types
frequently have different number of parameters.
5) Messages are being transferred using only 20 overloads where every
single overload doesn't seem to correspond to single type of message.
Overloads look like this (now you'll see the problem, I think):
---
int PushMsg(uchar msg, ushort, int);
int PushMsg(uchar msg, ushort, char *, int);
int PushMsg(uchar msg, ushort, ushort, char *, int);
int PushMsg(uchar msg, ushort, uchar, int);
int PushMsg(uchar msg, ushort, ushort, int);
int PushMsg(uchar msg, ushort, uchar, uchar, int);
int PushMsg(uchar msg, ushort, ushort, uchar, int);
int PushMsg(uchar msg, ushort, ushort, uchar, long, long, int);
int PushMsg(uchar,short,ushort,ushort,uchar,int);
int PushMsg(uchar,ushort,ushort,short,uchar,long,long,int);
Yeah, that is horrible WRT maintenance. It is easy to imagine a system
where you can construct a message and use simple polymorphism to
serialize it to the stream, e.g.
message.store(stream);
6) There are no structs defined for every message type.
7) In the end message data is being stored into one huge message
struct (with few dozens of members) which is not a union and seems to
contain every possible parameter.
8) Received messages are processed using huge switch/case.
Yes, but it does not seem that turning messages into commands will be
interesting (you mentioned command pattern). This is, I think, because
the idea of the command pattern is to execute the command at a later
stage or multiple times, which doesn't seem to be your case. (But
might be, further down the line; it's easy to imagine that messages
are put into some sort of execution queue; at that point, simple
application of the command pattern is warranted).
Goran.
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]