Problem
I am developing a network application and therefor need to specify the packets I want to send. I have a basic interfaces class and one for the packets that implement them.
public interface IPacket
{
}
public interface IServerPacket : IPacket
{
}
public interface IServerChatPacket : IServerPacket
{
ServerChatPacketType Type { get; }
}
public interface IServerAccountPacket : IServerPacket
{
ServerAccountPacketType Type { get; }
}
The specification of an IServerPacket
is needed for easier and faster serialization. Anyways, now I can use these sub-interfaces for chat server and account communication with the connected clients.
[Serializable]
public struct ServerChatIncomingMessagePacket : IServerChatPacket
{
public readonly string text;
public ServerChatPacketType
{
get { return ServerChatPacketType.IncomingMessage; }
}
public ServerChatIncomingMessagePacket(string text)
{
this.text = text;
}
}
[Serializable]
public struct ServerAccountAnswerLoginPacket : IServerAccountPacket
{
public readonly bool success;
public ServerAccountPacketType
{
get { return ServerAccountPacketType.AnswerLogin; }
}
public ServerAccountAnswerLoginPacket(bool success)
{
this.success = success;
}
}
Is this the right way to do it? Currently I really have no problems with this, but maybe I am using the interfaces or structs wrong? I am not new into C# or OOP but there may be some way that solves this in a different manner or with better performance (structs have a chance to hold much data which is unpractical for this type kind)?
Solution
One empty interface could be considered a code smell, but can sometimes be useful for reflection purposes. Having two is nonsensical. An interface defines a contract that the type has to adhere to. Where’s the contract here? I fail to see how these two types belong to the same inheritance tree. “Type” members also set off alarm bells in my head. They’re usually an indication that there are missing interfaces. You could then just inspect the actual type, if you really had to.
I would go with something more like this.
public interface IMessagePacket
{
string Message { get; }
}
public interface IStatusPacket
{
bool Status { get; }
}
[Serializable]
public struct ServerChatIncomingMessagePacket : IMessagePacket
{
public readonly string text;
public string Message
{
get { return text; }
}
public ServerChatIncomingMessagePacket(string text)
{
this.text = text;
}
}
[Serializable]
public struct ServerAccountAnswerLoginPacket : IStatusPacket
{
public readonly bool success;
public bool Status
{
get { return success; }
}
public ServerAccountAnswerLoginPacket(bool success)
{
this.success = success;
}
}
This allows us to define behavior of the types. We could easily implement an OutGoingMessagePacket
that implements IMessagePacket
, or a LogOffPacket
that implements IStatusPacket
.
On another note, in your original implementation, you never used the parameter you passed to ServerAccountAnswerLoginPacket
‘s constructor, or it’s related private readonly field.