Return to Question

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);
}

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;
        }
    }

If we check the NetworkServer class for what could cause itself to change, we can identify

 

• NetServer
• NetConnection
• DataMessageHandler
• NetIncomingMessage

 

because you use these as classes but not as interfaces. If you change the name of the class, or its implementation by removing or adding methods etc, you will also need to change the NetworkServer class.

 

I would like to suggest using interfaces here. Instead of injecting the NetPeerConfiguration into the constructor, you should just inject an INetServer interface.

Based on the comments and the chatting the problem is like that, that the NetServer and the related classes are out of your control.

 

This screams for the adapter pattern which you should place between your NetworkServer class and the NetServer class. The purpose here is not to help your application to understand the 3rd party server, but to hide deep down the code of the maybe ugly convertation of the NetIncomingMessage.

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);
}

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;
        }
    }

If we check the NetworkServer class for what could cause itself to change, we can identify

• NetServer
• NetConnection
• DataMessageHandler
• NetIncomingMessage

because you use these as classes but not as interfaces. If you change the name of the class, or its implementation by removing or adding methods etc, you will also need to change the NetworkServer class.

I would like to suggest using interfaces here. Instead of injecting the NetPeerConfiguration into the constructor, you should just inject an INetServer interface.

Based on the comments and the chatting the problem is like that, that the NetServer and the related classes are out of your control.

This screams for the adapter pattern which you should place between your NetworkServer class and the NetServer class. The purpose here is not to help your application to understand the 3rd party server, but to hide deep down the code of the maybe ugly convertation of the NetIncomingMessage.

This is an iterative review request of this other question I posted.

This is an iterative review request of this other question I posted.

As you can see, there are still a few changes that need made for each message I add, though they are changes I am ok with:

As you can see, there are still a few changes that need made for each message I add, though they are changes I am "OK" with: