CODEWITHSUNDEEP
c#design-patterns
user541978
user541978

Reputation: 23

Which design pattern should I use here

For learning purposes I am writing a socket based client/server application. I have designed a custom protocol to ensure I can process my packets correctly. While inspecting some older parts of code today I realized that my way of creating my data packets contains much redundant code.

I have different types of packets, e.g. ImagePacket, MessagePacket etc. The creation of all types differs only in minor stuff, the header and delimiter creation for example is the same.

To improve this, i came up with a solution like this (simplified):

abstract class Packet
{
 public Packet(object o)
 {
 MemoryStream memoryStream = new MemoryStream();         

 AddHeader(ref memoryStream);
 AddData(ref memoryStream, obj);
 AddDelimiter(ref memoryStream);
 _packetBytes = memoryStream.ToArray();

 memoryStream.Close();
}
protected abstract void AddData(ref MemoryStream ms, object obj);

The AddData method is implemented as abstract method and overridden in the concrete classes whereas AddHeader and AddDelimiter are defined in the abstract class Packet itself.

This works fine and I don't have as much duplicated code as before but I am not happy with passing an object to AddData because it is not made clear that i must not give a string to the ImagePacket constructor.

// correct
Packet myMsgPacket = new MessagePacket("hello world"); 
Packet myImagePacket = new ImagePacket(image);
// wrong, but will be compiled :(
Packet myChaosPacket = new ImagePacket("muaha you're doomed");

If I had to to implement a check for the correct data type being passed I would end up with tons of stupid code again. How can I achieve the reduction of duplicated code but also get rid of the problem mentioned above?

Upvotes: 2

Views: 374

Answers (4)

Basic
Basic

Reputation: 26756

This sounds like you need to use the factory pattern

Packet MyPacket = MyPacketFactory.CreatePacket(Data)

MyFactory.CreatePacket would then return an IPacket

Update: As per the comment below, I should've been clearer. Your factory can have a number of overloaded CreatePacket() methods which take different data types...

IPacket CreatePacket(Image Data) {}
IPacket CreatePacket(String Data) {}
IPacket CreatePacket(Exception Data) {}

and if you have multiple types of packet which contain just String (Eg Message packet, Status packet or similar) You could create an Enum indicating which string packet is desired...

IPacket CreatePacket(String Data, StringPacketTypesEnum PacketType) {}

Inside you packet factory, you can have common functions which deal with all the duplicated code - eg AddDelimiter() - This will keep your code DRY

Upvotes: 3

rodion
rodion

Reputation: 15029

I think your current solution is OK. You just need to narrow down the type of data passed to the ImagePacket constructor to Image type. For example:

class ImagePacket : Packet{
  public ImagePacket(Image img) : base(img){}
}

This will check that data is an image at copile-time. Of course, this might not work if you really have to pass an object type.

Upvotes: 0

Bruno Costa
Bruno Costa

Reputation: 2720

I would go to factory pattern that returns a interface.

Beside that, you don't need to use ref for reference types.

Upvotes: 0

EnabrenTane
EnabrenTane

Reputation: 7466

Ruby on Rails uses Validations to solve this.

If your addData() function called validate() on the input you should just have to override validate() in each subclass.

Upvotes: 0

Related Questions