Design Question

[Chris]

Hi

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

I have a base Message and derived classes eg LoginMessage (outbound),
LoginReplyMessa ge (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 LoginReplyMessa ge
from the message data.

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

Thanks for reading.

Chris

[Victor Bazarov]

"Chris" <ch***@tuxweb.o rg> wrote...

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

I have a base Message and derived classes eg LoginMessage (outbound),
LoginReplyMessa ge (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 LoginReplyMessa ge
from the message data.

- b: Pass the buffer to a 'factory' method which returns a message of
exact type, eg LoginReplyMessa ge, 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 LoginReplyMessa ge, since you can create either of the latter from
it, right? Then why have LoginMessage and LoginReplyMessa ge 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]

Victor Bazarov wrote:

"Chris" <ch***@tuxweb.o rg> wrote...

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

I have a base Message and derived classes eg LoginMessage (outbound),
LoginReplyMes sage (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 LoginReplyMessa ge
from the message data.

- b: Pass the buffer to a 'factory' method which returns a message of
exact type, eg LoginReplyMessa ge, 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 LoginReplyMessa ge, since you can create either of the latter from
it, right? Then why have LoginMessage and LoginReplyMessa ge 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<cha r> and various functions for manipulating the bits.

Each more specific Message, eg LoginMessage will contain more
properties, eg login_failed_re ason.

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

// 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<cha r> byteBuffer = conn.recieve();

LoginMessage msg = MsgFactory::Cre ateBuildMessage (byteBuffer);

Hope that makes it slightly clearer :)
Thanks for the help.

[David Rubin]

Chris wrote:

// 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<cha r> byteBuffer = conn.recieve(); LoginMessage msg = MsgFactory::Cre ateBuildMessage (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
LoginMessageRep ly.) 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::Cre ateBuildMessage (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<Lo ginMessage*>() to check whether
the pointer you get back from MsgFactory::Cre ateBuildMessage () has the right type.

LoginMsg *l = dynamic_cast<Lo ginMsg*>(MsgFac tory::CreateBui ldMessage(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::Cre ateBuildMessage (buf);

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

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

HTH,

/david

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown