Return to Answer

3. I'm not sure I understand why do you need different datagram interfaces for server and client. It looks like an overkill to me. The header should already identify where the datagram comes from. By using different interfaces and different baseclasses, you will have troubles implementing datagrams which could be sent by both server and client.

4. bool IsMessageValid(); method should be bool IsValid { get; } property.

5. DatagramNames implies, that this class contains a bunch of names, i.e. strings. But it is not the case. Maybe you should rename it to DatagramCodes or something.

6. DatagramFactory looks like an attemt to reinvent a DI container. :) You should consider using one of the existing frameworks, such as Casle.Windsor, Ninject, etc. It will allow you to register and resolve datagrams with a few lines of code.

7. Using attributes to store things such as protocol version can lead to troubles. Only use it if you are absolutely sure, that all your clients are always going to use up-to-date software and you are not going to need your server to be compatible with older versions of clients. Protocols tend to change, and it is usully a good idea to send a protocol version with a datagram. This extra byte can solve a lot of problems in a long run.

8.  while (server.IsRunning())
    {
    await Task.Delay(1);
    }
   

   don't use Task.Delay or Thread.Sleep in a loop with 1ms delay. This loop does no meaningful work whatsoever yet it can easily consume up to one CPU core. You should figure out a way to either put your main thread to work or put it to sleep until you explicitly signal it to wake up (by using WaitHandle, for example).

9. In you server code you use memoryStream.GetBuffer(). This method returns the entire inner array of memory stream, which is larger, then the datagram size. This will lead to larger traffic. To avoid this you should also save the actual datagram size, and use it instead of buffer.Length. Also, if you use default MemoryStream constructor, it will dynamically change the inner array size hitting both performance and memory usage. Ideally you want to use MemoryStream(byte[]) instead.

10. I would recommend against using sockets directly. They use pretty obsolete callback-ish api which was the way to go in older versions of C#, but nowadays it is pretty much replaced by TPL and async programming. Instead you should consider using the highest level of abstraction that is available. In your case its going to be UdpClient class. Which has a much cleaner API and is way easier to use and debug. Check this answer out for simple example.

3. I'm not sure I understand why do you need different datagram interfaces for server and client. It looks like an overkill to me. The header should already identify where the datagram comes from. By using different interfaces and different baseclasses, you will have troubles implementing datagrams which could be sent by both server and client.

4. bool IsMessageValid(); method should be bool IsValid { get; } property.

5. DatagramNames implies, that this class contains a bunch of names, i.e. strings. But it is not the case. Maybe you should rename it to DatagramCodes or something.

6. DatagramFactory looks like an attemt to reinvent a DI container. :) You should consider using one of the existing frameworks, such as Casle.Windsor, Ninject, etc. It will allow you to register and resolve datagrams with a few lines of code.

7. Using attributes to store things such as protocol version can lead to troubles. Only use it if you are absolutely sure, that all your clients are always going to use up-to-date software and you are not going to need your server to be compatible with older versions of clients. Protocols tend to change, and it is usully a good idea to send a protocol version with a datagram. This extra byte can solve a lot of problems in a long run.

8.  while (server.IsRunning())
    {
    await Task.Delay(1);
    }
   

   don't use Task.Delay or Thread.Sleep in a loop with 1ms delay. This loop does no meaningful work whatsoever yet it can easily consume up to one CPU core. You should figure out a way to either put your main thread to work or put it to sleep until you explicitly signal it to wake up (by using WaitHandle, for example).

9. In you server code you use memoryStream.GetBuffer(). This method returns the entire inner array of memory stream, which is larger, then the datagram size. This will lead to larger traffic. To avoid this you should also save the actual datagram size, and use it instead of buffer.Length. Also, if you use default MemoryStream constructor, it will dynamically change the inner array size hitting both performance and memory usage. Ideally you want to use MemoryStream(byte[]) instead.

10. I would recommend against using sockets directly. They use pretty obsolete callback-ish api which was the way to go in older versions of C#, but nowadays it is pretty much replaced by TPL and async programming. Instead you should consider using the highest level of abstraction that is available. In your case its going to be UdpClient class. Which has a much cleaner API and is way easier to use and debug. Check this answer out for simple example.

3. I'm not sure I understand why do you need different datagram interfaces for server and client. It looks like an overkill to me. The header should already identify where the datagram comes from. By using different interfaces and different baseclasses, you will have troubles implementing datagrams which could be sent by both server and client.

4. bool IsMessageValid(); method should be bool IsValid { get; } property.

5. DatagramNames implies, that this class contains a bunch of names, i.e. strings. But it is not the case. Maybe you should rename it to DatagramCodes or something.

6. DatagramFactory looks like an attemt to reinvent a DI container. :) You should consider using one of the existing frameworks, such as Casle.Windsor, Ninject, etc. It will allow you to register and resolve datagrams with a few lines of code.

7. Using attributes to store things such as protocol version can lead to troubles. Only use it if you are absolutely sure, that all your clients are always going to use up-to-date software and you are not going to need your server to be compatible with older versions of clients. Protocols tend to change, and it is usully a good idea to send a protocol version with a datagram. This extra byte can solve a lot of problems in a long run.

8.  while (server.IsRunning())
    {
    await Task.Delay(1);
    }
   

   don't use Task.Delay or Thread.Sleep in a loop with 1ms delay. This loop does no meaningful work whatsoever yet it can easily consume up to one CPU core. You should figure out a way to either put your main thread to work or put it to sleep until you explicitly signal it to wake up (by using WaitHandle, for example).

9. In you server code you use memoryStream.GetBuffer(). This method returns the entire inner array of memory stream, which is larger, then the datagram size. This will lead to larger traffic. To avoid this you should also remember the actual datagram size, and use it instead of buffer.Length. Also, if you use default MemoryStream constructor, it will dynamically change the inner array size hitting both performance and memory usage. Ideally you want to use MemoryStream(byte[]) instead.

10. I would recommend against using sockets directly. They use pretty obsolete callback-ish api which was the way to go in older versions of C#, but nowadays it is pretty much replaced by TPL and async programming. Instead you should consider using the highest level of abstraction that is available. In your case its going to be UdpClient class. Which has a much cleaner API and is way easier to use and debug. Check this answer out for simple example.

3. I'm not sure I understand why do you need different datagram interfaces for server and client. It looks like an overkill to me. The header should already identify where the datagram comes from. By using different interfaces and different baseclasses, you will have troubles implementing datagrams which could be sent by both server and client.

4. bool IsMessageValid(); method should be bool IsValid { get; } property.

5. DatagramNames implies, that this class contains a bunch of names, i.e. strings. But it is not the case. Maybe you should rename it to DatagramCodes or something.

6. DatagramFactory looks like an attemt to reinvent a DI container. :) You should consider using one of the existing frameworks, such as Casle.Windsor, Ninject, etc. It will allow you to register and resolve datagrams with a few lines of code.

7. Using attributes to store things such as protocol version can lead to troubles. Only use it if you are absolutely sure, that all your clients are always going to use up-to-date software and you are not going to need your server to be compatible with older versions of clients. Protocols tend to change, and it is usully a good idea to send a protocol version with a datagram. This extra byte can solve a lot of problems in a long run.

8.  while (server.IsRunning())
    {
    await Task.Delay(1);
    }
   

   don't use Task.Delay or Thread.Sleep in a loop with 1ms delay. This loop does no meaningful work whatsoever yet it can easily consume up to one CPU core. You should figure out a way to either put your main thread to work or put it to sleep until you explicitly signal it to wake up (by using WaitHandle, for example).

9. In you server code you use memoryStream.GetBuffer(). This method returns the entire inner array of memory stream, which is larger, then the datagram size. This will lead to larger traffic. To avoid this you should also save the actual datagram size, and use it instead of buffer.Length. Also, if you use default MemoryStream constructor, it will dynamically change the inner array size hitting both performance and memory usage. Ideally you want to use MemoryStream(byte[]) instead.

10. I would recommend against using sockets directly. They use pretty obsolete callback-ish api which was the way to go in older versions of C#, but nowadays it is pretty much replaced by TPL and async programming. Instead you should consider using the highest level of abstraction that is available. In your case its going to be UdpClient class. Which has a much cleaner API and is way easier to use and debug. Check this answer out for simple example.

3. I'm not sure I understand why do you need different datagram interfaces for server and client. It looks like an overkill to me. The header should already identify where the datagram comes from. By using different interfaces and different baseclasses, you will have troubles implementing datagrams which could be sent by both server and client.

4. bool IsMessageValid(); method should be bool IsValid { get; } property.

5. DatagramNames implies, that this class contains a bunch of names, i.e. strings. But it is not the case. Maybe you should rename it to DatagramCodes or something.

6. DatagramFactory looks like an attemt to reinvent a DI container. :) You should consider using one of the existing frameworks, such as Casle.Windsor, Ninject, etc. It will allow you to register and resolve datagrams with a few lines of code.

7. Using attributes to store things such as protocol version can lead to troubles. Only use it if you are absolutely sure, that all your clients are always going to use up-to-date software and you are not going to need your server to be compatible with older versions of clients. Protocols tend to change, and it is usully a good idea to send a protocol version with a datagram. This extra byte can solve a lot of problems in a long run.

8.  while (server.IsRunning())
    {
    await Task.Delay(1);
    }
   

   don't use Task.Delay or Thread.Sleep in a loop with 1ms delay. This loop does no meaningful work whatsoever yet it can easily consume up to one CPU core. You should figure out a way to either put your main thread to work or put it to sleep until you explicitly signal it to wake up (by using WaitHandle, for example).

I had no time to review actual client/server implementations. But I might do it later. Or maybe someone else will.

3. I'm not sure I understand why do you need different datagram interfaces for server and client. It looks like an overkill to me. The header should already identify where the datagram comes from. By using different interfaces and different baseclasses, you will have troubles implementing datagrams which could be sent by both server and client.

4. bool IsMessageValid(); method should be bool IsValid { get; } property.

5. DatagramNames implies, that this class contains a bunch of names, i.e. strings. But it is not the case. Maybe you should rename it to DatagramCodes or something.

6. DatagramFactory looks like an attemt to reinvent a DI container. :) You should consider using one of the existing frameworks, such as Casle.Windsor, Ninject, etc. It will allow you to register and resolve datagrams with a few lines of code.

7. Using attributes to store things such as protocol version can lead to troubles. Only use it if you are absolutely sure, that all your clients are always going to use up-to-date software and you are not going to need your server to be compatible with older versions of clients. Protocols tend to change, and it is usully a good idea to send a protocol version with a datagram. This extra byte can solve a lot of problems in a long run.

8.  while (server.IsRunning())
    {
    await Task.Delay(1);
    }
   

   don't use Task.Delay or Thread.Sleep in a loop with 1ms delay. This loop does no meaningful work whatsoever yet it can easily consume up to one CPU core. You should figure out a way to either put your main thread to work or put it to sleep until you explicitly signal it to wake up (by using WaitHandle, for example).

9. In you server code you use memoryStream.GetBuffer(). This method returns the entire inner array of memory stream, which is larger, then the datagram size. This will lead to larger traffic. To avoid this you should also remember the actual datagram size, and use it instead of buffer.Length. Also, if you use default MemoryStream constructor, it will dynamically change the inner array size hitting both performance and memory usage. Ideally you want to use MemoryStream(byte[]) instead.

10. I would recommend against using sockets directly. They use pretty obsolete callback-ish api which was the way to go in older versions of C#, but nowadays it is pretty much replaced by TPL and async programming. Instead you should consider using the highest level of abstraction that is available. In your case its going to be UdpClient class. Which has a much cleaner API and is way easier to use and debug. Check this answer out for simple example .