Is this proper OO design for C++?
- by user121917
I recently took a software processes course and this is my first time attempting OO design on my own. I am trying to follow OO design principles and C++ conventions. I attempted and gave up on MVC for this application, but I am trying to "decouple" my classes such that they can be easily unit-tested and so that I can easily change the GUI library used and/or the target OS. At this time, I have finished designing classes but have not yet started implementing methods.
The function of the software is to log all packets sent and received, and display them on the screen (like WireShark, but for one local process only). The software accomplishes this by hooking the send() and recv() functions in winsock32.dll, or some other pair of analogous functions depending on what the intended Target is. The hooks add packets to SendPacketList/RecvPacketList. The GuiLogic class starts a thread which checks for new packets. When new packets are found, it utilizes the PacketFilter class to determine the formatting for the new packet, and then sends it to MainWindow, a native win32 window (with intent to later port to Qt).1
Full size image of UML class diagram
Here are my classes in skeleton/header form (this is my actual code):
class PacketModel
{
protected:
std::vector<byte> data;
int id;
public:
PacketModel();
PacketModel(byte* data, unsigned int size);
PacketModel(int id, byte* data, unsigned int size);
int GetLen();
bool IsValid(); //len >= sizeof(opcode_t)
opcode_t GetOpcode();
byte* GetData(); //returns &(data[0])
bool GetData(byte* outdata, int maxlen);
void SetData(byte* pdata, int len);
int GetId();
void SetId(int id);
bool ParseData(char* instr);
bool StringRepr(char* outstr);
byte& operator[] (const int index);
};
class SendPacket : public PacketModel
{
protected:
byte* returnAddy;
public:
byte* GetReturnAddy();
void SetReturnAddy(byte* addy);
};
class RecvPacket : public PacketModel
{
protected:
byte* callAddy;
public:
byte* GetCallAddy();
void SetCallAddy(byte* addy);
};
//problem: packets may be added to list at any time by any number of threads
//solution: critical section associated with each packet list
class Synch
{
public:
void Enter();
void Leave();
};
template<class PacketType> class PacketList
{
private:
static const int MAX_STORED_PACKETS = 1000;
public:
static const int DEFAULT_SHOWN_PACKETS = 100;
private:
vector<PacketType> list;
Synch synch; //wrapper for critical section
public:
void AddPacket(PacketType* packet);
PacketType* GetPacket(int id);
int TotalPackets();
};
class SendPacketList : PacketList<SendPacket>
{
};
class RecvPacketList : PacketList<RecvPacket>
{
};
class Target //one socket
{
bool Send(SendPacket* packet);
bool Inject(RecvPacket* packet);
bool InitSendHook(SendPacketList* sendList);
bool InitRecvHook(RecvPacketList* recvList);
};
class FilterModel
{
private:
opcode_t opcode;
int colorID;
bool bFilter;
char name[41];
};
class FilterFile
{
private:
FilterModel filter;
public:
void Save();
void Load();
FilterModel* GetFilter(opcode_t opcode);
};
class PacketFilter
{
private:
FilterFile filters;
public:
bool IsFiltered(opcode_t opcode);
bool GetName(opcode_t opcode, char* namestr); //return false if name does not exist
COLORREF GetColor(opcode_t opcode); //return default color if no custom color
};
class GuiLogic
{
private:
SendPacketList sendList;
RecvPacketList recvList;
PacketFilter packetFilter;
void GetPacketRepr(PacketModel* packet);
void ReadNew();
void AddToWindow();
public:
void Refresh(); //called from thread
void GetPacketInfo(int id); //called from MainWindow
};
I'm looking for a general review of my OO design, use of UML, and use of C++ features. I especially just want to know if I'm doing anything considerably wrong.
From what I've read, design review is on-topic for this site (and off-topic for the Code Review site).
Any sort of feedback is greatly appreciated. Thanks for reading this.
