Implementing Dynamic Network Message Handling

I'm trying to handle network messages in a dynamic, and effective way, though I feel I've overthought it again. As of now, I have a very structured approach, but it feels as if it's a lot of overhead for such a simple process.

The other issue, is that if I start making more types of Login messages, then I will have many small classes floating around. Is this an appropriate design? Should I allow micro-classes here?

This code stems from the code mentioned over on this other question. Though, I am working on a different portion of it now. That said, most (if not all) of the suggestions from that question have been implemented in some manner.

I have rewritten the entire NetworkServer as follows:

```
public class NetworkServer
{
private readonly NetServer _MainNetServer;
private readonly HashSet _ConnectedClients = new HashSet();
private readonly ILogger _Logger;
private readonly DataMessageHandler _DataMessageHandler;

public NetworkServer(NetPeerConfiguration serverConfiguration, ILogger logger)
{
_Logger = logger;

_Logger.LogInformation("Initializing server...");
_MainNetServer = new NetServer(serverConfiguration);
_DataMessageHandler = new DataMessageHandler(_Logger);
}

public Task RunServer()
{
return Task.Run(() =>
{
_MainNetServer.Start();
_Logger.LogImportant("Server starting on socket {0}...", _MainNetServer.Socket.LocalEndPoint);
DateTime started = DateTime.UtcNow;

//_Logger.LogImportant(_Logger.FormatMessage("The server was started on {0} at {1}.", started.ToString("dd-MM-yyyy"), started.ToString("HH:mm:ss.fffffff")));
_Logger.LogImportant("The server was started at: {0}", started.ToString("O"));

while (true)
{
NetIncomingMessage msg;
while ((msg = _MainNetServer.ReadMessage()) != null)
{
switch (msg.MessageType)

I really don't like this enum and "type" member.

public enum DataMessageType
{
    Login,
}

public interface IDataMessage
{
    ILogger Logger { get; }
    DataMessageType Type { get; }

    void WriteNetworkMessage(NetOutgoingMessage message);
    void ReadNetworkMessage(NetIncomingMessage message);
}

The idea behind polymorphism is that so long as the type adheres to an interface, the client code just doesn't care what the concrete type really is. As you've noticed, this implementation requires you adjust an enum, and at least two other classes every time you add a new child type of IDataMessage. This isn't how it's supposed to work.

Right now, the only thing HandleMessage does is turn a NetIncomingMessage into an IDataMessage. This is good. It's a single responsibility, but the name isn't the greatest. It's a factory class that adapts an outside interface into an internal one. Consider a rename for it, but over all it's good. You need a class that is responsible for determining which type of IDataMessage to create based on the state of NetIncomingMessage. It makes sense to have to update this class anytime you create a new implementation of IDataMessage. Don't worry about that.

This here is where things get not so good.

private void ProcessDataMessage(NetIncomingMessage message)
    {
        _Logger.LogInformation("Data recieved from: {0}, payload size: {1} bytes", message.SenderConnection.RemoteEndPoint, message.LengthBytes);

        IDataMessage messageResult = _DataMessageHandler.HandleMessage(message);

        switch (messageResult.Type)
        {
            case DataMessageType.Login:
                // Do login-ey things
                string username = ((LoginMessage)messageResult).Username;
                byte[] password = ((LoginMessage)messageResult).Password;
                break;
        }
    }

This is where it just shouldn't matter what type of IDataMessage we have. The code should just work so long as the message returned from the handler adheres to the interface.

We can fix this by letting each IDataMessage define it's own behavior. First, let's adjust the interface a little bit.

public interface IDataMessage
{
    ILogger Logger { get; }

    void WriteNetworkMessage(NetOutgoingMessage message);
    void ReadNetworkMessage(NetIncomingMessage message);

    void Process(); // void may not be appropriate, I don't have enough context to say.
}

Note that I removed the "type" enum. If we ever really just have to know exactly what concrete type we're working with, we can call typeof() instead. Now we move the interesting code that used to be in ProcessDataMessage to LoginMessage.

public class LoginMessage : IDataMessage
{
    //...

    void Process()
    {
        // processing code from ProcessDataMessage
    }

    //...
}

Then we adjust the original method accordingly, ending up with much simpler code here.

private void ProcessDataMessage(NetIncomingMessage message)
    {
        _Logger.LogInformation("Data recieved from: {0}, payload size: {1} bytes", message.SenderConnection.RemoteEndPoint, message.LengthBytes);

        IDataMessage messageResult = _DataMessageHandler.HandleMessage(message);

        messageResult.Process();
    }

See how it doesn't matter what type we get back from the factory? All of the differentiating logic is stored and implemented in the specific concrete implementations.

StackExchange Code Review Q#96592, answer score: 6