views:

43

answers:

2

I am designing a server which is used in UDP communication using MFC. I have the following classes

  • CMyDlialog - Take care of User interface
  • CController - Act as an Mediator between all the classes
  • CProtocolManager - Take care of Encoding/Decoding msgs (This is a static class)
  • CConnectionManager - Take care of UDP connection, Sending, Receiving

I am creating an object of CConnectionManager as a member variable in CController and object of CController as member variable in CMyDialog.

When user type something and presses send, I am calling a method in CControler which will call a method in CProtocolManager to construct the packet and call the CConnectionManager method to send it.

When i receive some data, it is handled in a thread of CConnectionManager. There i am creating a local object of CController and call a method, which will pass the data to CProtocolManager to decode.

Now i want to inform the UI about the data, how should the CControler do it? i can post a message to the UI by making the main dialog handle global, is that a correct approach.

Also tell me whether this design is correct one.

Thanks in advance

A: 

It seems you are doing it all in a single thread. Since it takes time for packets to go back and forth, it's advised to make do to time-consuming job in a different thread, and post status/results the UI thread. (Or the UI will be frozen).

Am
Actually i am listening for connections and processing the request in a different thread that is spawned by CConnectionManager.
Jeeva
A: 

I think designs are never right or wrong, but you can rate them according to some principles that many people consider to be "good" (see the SOLID principles).

Your sending approach sounds reasonable, but making the Dialog global for receiving is definitely considered "not so good". See the hollywood principle. I suggest you better pass a callback method to your connection manager that is a method of CController (which then lets CProtocolManager decode it and calls another callback method from the dialog). If callbacks are not what you want you can define AbstractBaseClasses (ABC) like this "AbstractMessageReceiver" with a method

 virtual void receive(const char*, int length) = 0;

You can then implement this ABC in CProtocolManager pass this as a "AbstractMessageReceiver*" to CConnectionManager which then calls

m_myMessageReceiver->receive(m_buffer, m_length);

or something similar.

This approach reduces the coupling, is much more testable and increases the reusability.

Furthermore, I agree with Am that you should think about your threading model!

Philipp
So i should pass the AbstractMessageReceiver* to the thread function that receives the data and pass that to the controller which will call the actual recieve method?
Jeeva
you could do that, but I wonder if you really need to create a local controller or if you can just pass your existing controller to the connection manager? Then you can pass the AbstractMessageReceiver to the controller and the connection manager doesn't need to worry about it.
Philipp