Design Question

[Chris]

Hi

My application reads writes/messages from a socket. It contains inbound
and outbound std::queue<Message*> queues.

I have a base Message and derived classes eg LoginMessage (outbound),
LoginReplyMessage (inbound).

My question is when creating an inbound message from the raw data which
of the following approaches is better?

- a: Directly create object of Message type, add onto the inbound
message queue . Then when using it later create a new LoginReplyMessage
from the message data.

- b: Pass the buffer to a 'factory' method which returns a message of
exact type, eg LoginReplyMessage, using a add onto inbound message queue
(upcasting). When processing downcast to appropriate type and use.

Thanks for reading.

Chris

 

[Victor Bazarov]

My application reads writes/messages from a socket. It contains inbound
and outbound std::queue<Message*> queues.

I have a base Message and derived classes eg LoginMessage (outbound),
LoginReplyMessage (inbound).

My question is when creating an inbound message from the raw data which
of the following approaches is better?

- a: Directly create object of Message type, add onto the inbound
message queue . Then when using it later create a new LoginReplyMessage
from the message data.

- b: Pass the buffer to a 'factory' method which returns a message of
exact type, eg LoginReplyMessage, using a add onto inbound message queue
(upcasting). When processing downcast to appropriate type and use.

I don't understand, probably. So, each 'Message' already contains
all the necessary information to either be a LoginMessage or
a LoginReplyMessage, since you can create either of the latter from
it, right? Then why have LoginMessage and LoginReplyMessage at all?

If you can have 'Message' object distinguish which one of two derived
types it is, there is no sense to have derived types. Therefore your
problem doesn't exist.

 

[Chris]

I don't understand, probably. So, each 'Message' already contains
all the necessary information to either be a LoginMessage or
a LoginReplyMessage, since you can create either of the latter from
it, right? Then why have LoginMessage and LoginReplyMessage at all?

If you can have 'Message' object distinguish which one of two derived
types it is, there is no sense to have derived types. Therefore your
problem doesn't exist.

Each Message contains the information needed to send over the Socket, ie
a std::vector<char> and various functions for manipulating the bits.

Each more specific Message, eg LoginMessage will contain more
properties, eg login_failed_reason.

// sending is fine
LoginMsg l(username, password)
conn.send(l.getPacket());

// MY question is when receiving

// ** 2 Different Alternatives Below
// 1
Message msg = conn.reveive()
int type = msg.getType();

switch(type) {
case LOGIN:
// create login message?
}

// 2
std::vector<char> byteBuffer = conn.recieve();

LoginMessage msg = MsgFactory::CreateBuildMessage(byteBuffer);

Hope that makes it slightly clearer
Thanks for the help.

 

[David Rubin]

// MY question is when receiving

// ** 2 Different Alternatives Below
// 1
Message msg = conn.reveive()
int type = msg.getType();

switch(type) {
case LOGIN:
// create login message?
}

// 2
std::vector<char> byteBuffer = conn.recieve();

LoginMessage msg = MsgFactory::CreateBuildMessage(byteBuffer);

The problem with (2) is that you are assuming the message is a LoginMessage. (I
guess this is on the server, otherwise the client would do the same for
LoginMessageReply.) I'm not sure why this is guaranteed. For example, (1) does
not attempt to identify the message as a particular type.

From my perspective, (2) is nicer, but should be expressed as

Message *msg = MsgFactory::CreateBuildMessage(buf);

Then, you can use polymorphism to get the correct behavior out of msg; e.g.,

if(msg->error()){
std::cerr << "error: " << msg->tostring() << std::endl;
return 1;
}

yielding

error: username/password not recognized

OTOH, you may need to cast msg to an appropriate type depending on your server
state machine in order to get certain values. For example, if you are in the
"login request" state, you may need

l->username()
l->password()

In this case, I think you can use dynamic_cast<LoginMessage*>() to check whether
the pointer you get back from MsgFactory::CreateBuildMessage() has the right type.

LoginMsg *l = dynamic_cast<LoginMsg*>(MsgFactory::CreateBuildMessage(buf);

if(l == 0){
// error
}

Another way to do this is to check

msg->type() == MSG_LOGIN_REQ

WLOG regarding your type definitions. This doesn't make much use of the type
system, but the information must already be there in order for the factory to work.

A simpler scheme might be to use specific message poninters to directly access
message data (rather than wrap messages in accessor classes), and simply retain
the Message type for encapsulating generic message operations [error(),
tostring(), packetize(), etc]. This forces you to think carefully about the
structure of the protocol messages since they are all related now by the
placement and encoding of certain fields (e.g., "type" is the first byte,
"error" is the second byte, error strings are ASCII payloads following the error
byte). In any case, this might look like

Message *msg = MsgFactory::CreateBuildMessage(buf);

if(msg->type() == MSG_LOGIN){
LoginMsg *l = msg->payload();

if(login(l->username, l->password) == false){
// error
}
// ...
}

HTH,

/david