Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Flexible design to replace a switch statement

Tags:

c++

networking

I am working on a networking program and designing a Linux server using C++. This is quite straightforward to design the basic structure. I have a packet definition with a header that has a fixed size.

typedef enum{
     PACKET_LOGIN_REQ = 1,
     PACKET_LOGIN_RES,
     PACKET_STORE_REQ,
     PACKET_STORE_RES
}PACKET_TYPES;

typedef struct {
     PACKET_TYPES type;
     short bodySize,
     long long deviceId
}HEADER;

.
.

/*more definitions here*/

typedef struct{
     HEADER head;
     union BODY{
          LOGIN_REQ loginReq;
          LOGIN_RES loginRes;
          .
          .
          more types
     }
}

Whenever I added more packet types I would have to modify the switch statement to add more cases to handle the newly added packets.

I am using the union type so I don't have to change the entire packet structure. Instead I can add the newly added packet types into the union structure.

However, when I am trying to parse the raw data to put into the packet using a switch statement then I have to add each switch statement every time.

I think this is not a good design pattern and I was wondering how I can design the structure in a more flexible way.

Is there a better way to handle this (better design pattern)? What about related tutorials or references?

like image 317
codereviewanskquestions Avatar asked Jun 15 '11 07:06

codereviewanskquestions


2 Answers

Since your code is C style and not C++ style code, I will give you the C answer.

Use function pointers. Something like:

typedef int (*ParseFunc)(const char *, len, struct MyStruct);

typedef struct PacketParser_
{
    PACKETType type;
    ParseFunc parseFunc;
} PacketParser;

const PacketParser[] parser = 
{
   { PACKET_LOGIN_REQ, LoginReqParser }
   { PACKET_LOGIN_RES, LoginResParser }
   .
   .
   .
}

You then iterate through all the packet types and call the corresponding function. Whenever you get a new packet type, you just add another line to your PacketParser function.

If you are using C++ you are better off using polymorphism.

like image 41
doron Avatar answered Oct 01 '22 09:10

doron


You should use classes and instead of switch use a polymorphic call.

Something like that (pseudo-codish):

class Packet{
public:
   Packet();
   virtual ~Packet();
   virtual HandleMe()=0;
protected:
   // fields 
}

PacketTypeOne: public Packet{
public:
  virtual HandleMe(); // implementation for type 1
}

PacketType2: public Packet{
public:
  virtual HandleMe(); // implementation for type 2
}

PacketTypeX: public Packet{
public:
  virtual HandleMe(); // implementation for type X
}

and where you get the packets:

...
Packet* newPacket = PacketFactory::CreateNewPacket(packetTypeX); // factory will create
              // the right instance.
...
// now handle it:
newPacket->HandleMe();
...

Done.

like image 170
littleadv Avatar answered Oct 01 '22 09:10

littleadv