4
\$\begingroup\$

I created a TCP Server and Client and I really would like to know if its any good in regards of performance and code quality / safety. I hightlight the server and client class here. If you need more insight in the Helper classes I could share them as well. One thing I know is creating threads for each client is not really good and I would like to know at what number of sockets connected it really makes a difference.

TCPServer.cs

/// <summary>
/// Multithreaded TCP Server
/// </summary>
public class TCPServer : TCPBase {

    /// <summary>
    /// Max Package length, default 100MB
    /// </summary>
    public static int MaxPackageLength { get; set; } = 107374182;

    /// <summary>
    /// List of all clients currently connected to the server
    /// </summary>
    public List<TCPServerClient> ClientsList { get; protected set; } = new List<TCPServerClient>();

    /// <summary>
    /// Backlog to use, only change before start server
    /// </summary>
    public int Backlog { get; set; } = 500;

    /// <summary>
    /// Size of the uid of a client
    /// </summary>
    public uint UIDLength { get; set; } = 12;

    /// <summary>
    /// Whether clients need to make a initial handshake
    /// </summary>
    public bool RequireHandshake { get; set; } = true;

    /// <summary>
    /// Is logging enabled
    /// </summary>
    public bool Logging { get; set; } = true;

    /// <summary>
    /// Whether pinging is enabled
    /// </summary>
    public bool Pinging { get; set; } = true;

    /// <summary>
    /// Thread to handle management such as kick clients with no handshake
    /// </summary>
    public Thread ManagementThread { get; private set; }

    /// <summary>
    /// Thread to handle pinging and rtt
    /// </summary>
    public Thread PingThread { get; private set; }

    /// <summary>
    /// Management sleep time in ms
    /// </summary>
    public int ManagementSleep { get; set; } = 20000;

    /// <summary>
    /// Ping sleep time in ms
    /// </summary>
    public int PingSleep { get; set; } = 5000;

    /// <summary>
    /// Time span until clients are kicked without handshake
    /// </summary>
    public TimeSpan HandshakeTimeout { get; set; } = new TimeSpan(0, 0, 40);

    /// <summary>
    /// Dictionary containing all clients identified by their uid
    /// </summary>
    public Dictionary<string, TCPServerClient> ClientsDict { get; protected set; } = new Dictionary<string, TCPServerClient>();

    /// <summary>
    /// Message Event Handler
    /// </summary>
    /// <param name="client"></param>
    /// <param name="message"></param>
    public delegate void MessageEventHandler(TCPServerClient client, TCPMessage message);

    /// <summary>
    /// Message Event, called if a client sent a message
    /// </summary>
    public event MessageEventHandler OnMessage;

    /// <summary>
    /// Connected Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void ConnectedEventHandler(TCPServerClient client);

    /// <summary>
    /// Connected Event, called if a new client is successfully connected
    /// </summary>
    public event ConnectedEventHandler OnConnected;

    /// <summary>
    /// Disconnected Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void DisconnectedEventHandler(TCPServerClient client);

    /// <summary>
    /// Disconnected Event, called if a client is disconnected
    /// </summary>
    public event DisconnectedEventHandler OnDisconnected;

    /// <summary>
    /// No Handshaked Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void NoHandshakeEventHandler(TCPServerClient client);

    /// <summary>
    /// No Handshake Event, called if client fails to provide correct init package
    /// </summary>
    public event NoHandshakeEventHandler OnNoHandshake;

    /// <summary>
    /// Timeout Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void TimeoutEventHandler(TCPServerClient client);

    /// <summary>
    /// Timeout Event, called if client is timed out
    /// </summary>
    public event TimeoutEventHandler OnTimeout;

    /// <summary>
    /// Kick Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void KickEventHandler(TCPServerClient client);

    /// <summary>
    /// Kick Event, called if client was kicked
    /// </summary>
    public event KickEventHandler OnKick;

    /// <summary>
    /// Handshake Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void HandshakeHandler(TCPServerClient client);

    /// <summary>
    /// Handshake Event
    /// </summary>
    public event HandshakeHandler OnHandshake;

    /// <summary>
    /// Default constructor, default uses ipv4 address
    /// </summary>
    /// <param name="port"></param>
    /// <param name="ssl"></param>
    public TCPServer(ushort port = 27789, X509Certificate2 ssl = null, IPAddress address = null) {

        if (Logging)
            Logger.Write("REGION", "TCP Server Constructor");

        SSL = ssl;
        Port = port;

        if (Logging) {
            Logger.Write("INIT", "Setting Port: " + Port);
            Logger.Write("INIT", "Setting SSL: " + SSL);
        }

        if (address == null) {

            var host = Dns.GetHostEntry(Dns.GetHostName());

            foreach (IPAddress adr in host.AddressList) {
                if (adr.AddressFamily == AddressFamily.InterNetwork) {
                    Address = adr;
                    break;
                }
            }

            if (Address == null) {
                Address = host.AddressList[0];
            }

        } else {
            Address = address;
        }

        if (Logging) {
            Logger.Write("INIT", "Using Address: " + Enum.GetName(typeof(AddressFamily), Address.AddressFamily) + "//" + Address.ToString());
        }

        Socket = new Socket(AddressFamily.InterNetworkV6, SocketType.Stream, ProtocolType.Tcp);
        Socket.SetSocketOption(SocketOptionLevel.IPv6, SocketOptionName.IPv6Only, false);
        Socket.Bind(new IPEndPoint(Address, Port));

        Running = false;

    }
    
    /// <summary>
    /// Kicks user
    /// </summary>
    /// <param name="client"></param>
    public void Kick(TCPServerClient client) {

        RemoveClient(client, TCPDisconnectType.Kick);

    }

    /// <summary>
    /// Start the server
    /// </summary>
    /// <returns></returns>
    public bool Start() {

        if (Logging) {
            Logger.Write("REGION", "Method [Start]");
        }

        if ((ListenThread == null || !ListenThread.IsAlive) && !Running) {

            if (Logging) {
                Logger.Write("SUCCESS", "Starting server");
            }

            ListenThread = new Thread(() => Listen());
            ManagementThread = new Thread(Management);
            PingThread = new Thread(Ping);

            Running = true;

            ListenThread.Start();
            ManagementThread.Start();
            PingThread.Start();

            return true;

        }

        if (Logging) {
            Logger.Write("FAILED", "Starting server");
        }

        return false;

    }

    /// <summary>
    /// Stop the server
    /// </summary>
    public void Stop() {

        if (Logging) {
            Logger.Write("REGION", "Method [Stop]");
        }

        if (Logging) {
            Logger.Write("INFO", "Stopping server");
        }

        Running = false;
        
        lock (ClientsList)
        lock(ClientsDict) {

            for(int e = ClientsList.Count - 1; e >= 0; e--) {

                TCPServerClient client = ClientsList[e];

                RemoveClient(client, TCPDisconnectType.Disconnect, e);

            }

        }

        try {

            Socket.Shutdown(SocketShutdown.Both);
            Socket.Close();

        } catch(Exception er) {

        }

        ManagementThread.Join();
        ListenThread.Join();

        ListenThread = new Thread(() => Listen());
        ManagementThread = new Thread(Management);

        Socket = new Socket(AddressFamily.InterNetworkV6, SocketType.Stream, ProtocolType.Tcp);
        Socket.SetSocketOption(SocketOptionLevel.IPv6, SocketOptionName.IPv6Only, false);
        Socket.Bind(new IPEndPoint(Address, Port));
        
        if (Logging) {
            Logger.Write("INFO", "Stopped server");
        }

    }

    /// <summary>
    /// Removes a client from the server
    /// </summary>
    /// <param name="client"></param>
    /// <param name="type"></param>
    public void RemoveClient(TCPServerClient client, TCPDisconnectType type = TCPDisconnectType.Disconnect, int pos = -1) {

        if (Logging) {
            Logger.Write("REGION", "Method [RemoveClient]");
        }

        if (type == TCPDisconnectType.NoHandshake) {

            if (Logging) {
                Logger.Write("INFO", "Client no handshake: " + client.UID);
            }
            OnNoHandshake?.Invoke(client);

        } else if (type == TCPDisconnectType.Disconnect) {

            if (Logging) {
                Logger.Write("INFO", "Client disconnect: " + client.UID);
            }
            OnDisconnected?.Invoke(client);

        } else if (type == TCPDisconnectType.Timeout) {

            if (Logging) {
                Logger.Write("INFO", "Client timeout: " + client.UID);
            }
            OnTimeout?.Invoke(client);

        } else if (type == TCPDisconnectType.Kick) {

            if (Logging) {
                Logger.Write("INFO", "Client kick: " + client.UID);
            }
            OnKick?.Invoke(client);

        }

        lock (ClientsDict) ClientsDict.Remove(client.UID);
        lock (ClientsList) {

            if(pos >= 0) {

                ClientsList.RemoveAt(pos);

            } else {

                for (int e = ClientsList.Count - 1; e >= 0; e--) {

                    if (ClientsList[e].UID == client.UID) {
                        if (Logging) {
                            Logger.Write("INFO", "Client found in ClientsList: " + client.UID);
                        }
                        ClientsList.RemoveAt(e);
                        break;
                    }

                }

            }

        }

        try {

            client.Socket.Shutdown(SocketShutdown.Both);
            client.Socket.Close();

        } catch (Exception e) {

            if (Logging) {
                Logger.Write("FAILED", "Socket shutdown/close", e);
            }

        }

    }

    protected void Ping() {

        while(Running && Pinging) {

            Thread.Sleep(PingSleep);

            lock(ClientsList) {
                lock(ClientsDict) {

                     for (int e = ClientsList.Count - 1; e >= 0; e--) {

                        TCPServerClient client = ClientsList[e];

                        try {

                            using (IOStream stream = new IOStream()) {

                                stream.WriteDouble(client.RTT);
                                stream.WriteString(DateTime.UtcNow.ToString("O"));

                                byte[] arr = stream.ToArray();

                                client.Send(new TCPMessage() {
                                    Code = TCPMessageCode.Ping,
                                    Content = arr
                                });

                            }

                        } catch (Exception er) {

                            RemoveClient(client, TCPDisconnectType.Timeout);

                        }

                    }

                }
            }

        }

    }

    /// <summary>
    /// Management to handle automated kicks etc
    /// </summary>
    protected void Management() {

        while (Running) {

            Thread.Sleep(ManagementSleep);

            lock (ClientsList) {
                lock (ClientsDict) {

                    for (int e = ClientsList.Count - 1; e >= 0; e--) {

                        TCPServerClient c = ClientsList[e];

                        if ((DateTime.Now - c.Joined) > HandshakeTimeout
                            && RequireHandshake && !c.DoneHandshake) {

                            RemoveClient(c, TCPDisconnectType.NoHandshake);

                        }

                    }
                }
            }

        }

    }

    /// <summary>
    /// Listen for new connections
    /// </summary>
    protected void Listen() {

        if (Logging) {
            Logger.Write("REGION", "Method [Listen]");
        }

        Socket.Listen(Backlog);

        if (Logging) {
            Logger.Write("INFO", "Start listening for clients");
        }

        while (Running) {

            Socket socket = Socket.Accept();

            if (Logging) {
                Logger.Write("INFO", "New socket connected");
            }

            TCPServerClient client = new TCPServerClient(
                socket, RandomGen.GenRandomUID(ClientsDict, UIDLength));

            client.Joined = DateTime.Now;

            Thread clientThread = new Thread(() => ListenClient(client));
            client.Thread = clientThread;

            clientThread.Start();

            if (Logging) {
                Logger.Write("INFO", "Created client and started thread");
            }

        }

    }

    /// <summary>
    /// Listen for new messages of individual clients
    /// </summary>
    /// <param name="client"></param>
    protected void ListenClient(TCPServerClient client) {

        if (Logging) {
            Logger.Write("REGION", "Method [ListenClient]");
        }

        using (Stream ns = GetStream(client)) {

            client.Stream = ns;

            client.Writer = new TCPWriter(ns);
            client.Reader = new TCPReader(ns);

            if (Logging) {
                Logger.Write("INFO", "Created stream, writer and reader for client: " + client.UID);
            }

            lock (ClientsList) ClientsList.Add(client);
            lock(ClientsDict) ClientsDict.Add(client.UID, client);
            OnConnected?.Invoke(client);

            if (RequireHandshake) {

                TCPMessage message = client.Reader.Read(client);

                if (message == null || message.Code != TCPMessageCode.Init
                    || message.Content.Length > 10) {
                    RemoveClient(client, TCPDisconnectType.NoHandshake);
                    return;
                }

                if (Logging) {
                    Logger.Write("SUCCESS", "Handshake: " + client.UID);
                }

                client.DoneHandshake = true;

                client.Send(new TCPMessage() {
                    Code = TCPMessageCode.Init,
                    Content = new byte[] { 0, 1, 0 }
                });

                OnHandshake?.Invoke(client);

            }

            while (Running && ClientsDict.ContainsKey(client.UID)) {

                TCPMessage message = client.Reader.Read(client);

                if(message == null) {
                    RemoveClient(client, TCPDisconnectType.Timeout);
                    return;
                }

                if (Logging) {
                    Logger.Write("INFO", "New message " + Enum.GetName(typeof(TCPMessageCode), message.Code) + " from user: " + client.UID);
                }

                if (message.Code == TCPMessageCode.Close) {
                    RemoveClient(client, TCPDisconnectType.Disconnect);
                }

                if(message.Code == TCPMessageCode.Pong) {
                    HandlePong(message);
                    continue;
                }

                if(message.Code == TCPMessageCode.Message)
                    OnMessage?.Invoke(client, message);

            }

        }

    }

    /// <summary>
    /// Handle pong and rtt
    /// </summary>
    /// <param name="message"></param>
    protected void HandlePong(TCPMessage message) {

        try {

            string strDate = Encoding.UTF8.GetString(message.Content);
            DateTime time = DateTime.Parse(strDate, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind);

            message.Client.RTT = ((DateTime.UtcNow.Ticks - time.Ticks) / 10000);

        } catch(Exception er) {

            if (Logging) {
                Logger.Write("FAILED", "Socket RTT failed", er);
            }

        }

    }

    /// <summary>
    /// Get appropiate stream of socket
    /// </summary>
    /// <param name="client"></param>
    /// <returns></returns>
    protected Stream GetStream(TCPServerClient client) {

        Stream stream = new NetworkStream(client.Socket);

        if (SSL == null) {

            return stream;

        }

        try {

            SslStream sslStream = new SslStream(stream, false);
            var task = sslStream.AuthenticateAsServerAsync(SSL, false, SSLProtocol, true);
            task.Start();
            task.Wait();

            return sslStream;

        } catch (Exception e) {

            return null;

        }

    }

}

TCPClient.cs

/// <summary>
/// TCPClient used to conenct to and communicate with tcp server
/// </summary>
public class TCPClient : TCPBase {

    /// <summary>
    /// Is logging enabled
    /// </summary>
    public bool Logging { get; set; } = true;

    /// <summary>
    /// Time until next reconnect try in ms
    /// </summary>
    public int ReconnectSleep { get; set; } = 2000;

    /// <summary>
    /// Whether or not a handshake is required
    /// </summary>
    public bool RequireHandshake { get; set; } = true;

    /// <summary>
    /// Enabled raw communcation, only use if you know what you are doing
    /// </summary>
    public bool RawSocket { get; set; } = false;

    /// <summary>
    /// Stream of the socket
    /// </summary>
    public Stream Stream { get; private set; }

    /// <summary>
    /// Writer to the stream
    /// </summary>
    public TCPWriter Writer { get; private set; }

    /// <summary>
    /// Reader to the stream
    /// </summary>
    public TCPReader Reader { get; private set; }

    /// <summary>
    /// RTT of connection
    /// </summary>
    public double RTT { get; private set; }

    /// <summary>
    /// Message Event Handler
    /// </summary>
    /// <param name="message"></param>
    public delegate void MessageEventHandler(TCPMessage message);

    /// <summary>
    /// Message Event, called if the server sent a message
    /// </summary>
    public event MessageEventHandler OnMessage;

    /// <summary>
    /// Connected Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void ConnectedEventHandler();

    /// <summary>
    /// Connected Event, called if the client connected to the server
    /// </summary>
    public event ConnectedEventHandler OnConnected;

    /// <summary>
    /// Handshake Event Handler
    /// </summary>
    /// <param name="client"></param>
    public delegate void HandshakeEventHandler();

    /// <summary>
    /// Handshake Event, called if the client successfully done the handshake
    /// </summary>
    public event HandshakeEventHandler OnHandshake;

    /// <summary>
    /// Disconnected Event Handler
    /// </summary>
    public delegate void DisconnectedEventHandler();

    /// <summary>
    /// Disconnected Event, called if client was disconnected 
    /// </summary>
    public event DisconnectedEventHandler OnDisconnected;

    /// <summary>
    /// Default constructor
    /// </summary>
    /// <param name="address"></param>
    /// <param name="port"></param>
    /// <param name="ssl"></param>
    public TCPClient(string address = "localhost", ushort port = 27789, bool logging = true) {

        Logging = logging;

        if(Logging) {
            Logger.Write("REGION", "TCP Client Constructor");
        }

        IPAddress adr = null;

        if(!IPAddress.TryParse(address, out adr)) {
            throw new Exception("IPAddress not recognizable");
        }

        Address = adr;
        AddressString = address;
        Port = port;

        if (Logging) {
            Logger.Write("INIT", "Setting Port: " + Port);
            Logger.Write("INIT", "Setting SSL: " + SSL);
            Logger.Write("INIT", "Using Address: " + Enum.GetName(typeof(AddressFamily), Address.AddressFamily) + "//" + Address.ToString());
        }

        Running = false;
        InitHandlers();

    }

    /// <summary>
    /// Sends a message to the server
    /// </summary>
    /// <param name="message"></param>
    public void Send(TCPMessage message) {

        Writer.Write(message);

    }

    /// <summary>
    /// Listen for incomming messages
    /// </summary>
    protected void Listen() {

        using(Stream = GetStream()) {

            Writer = new TCPWriter(Stream);
            Reader = new TCPReader(Stream);

            if (Logging) {
                Logger.Write("SUCCESS", "Connected to the server");
            }

            OnConnected?.Invoke();

            if (RequireHandshake) {

                byte[] rand = new byte[10];
                RandomGen.Random.NextBytes(rand);

                TCPMessage init = new TCPMessage() {
                    Code = TCPMessageCode.Init,
                    Content = rand
                };

                if (Logging) {
                    Logger.Write("INFO", "Sending handshake");
                }

                Send(init);

            }

            while (Running) {

                TCPMessage message = Reader.Read(Socket);

                if(message == null) {
                    Running = false;
                    OnDisconnected?.Invoke();
                    continue;
                }

                if (message.Code == TCPMessageCode.Init) {
                    if (Logging) {
                        Logger.Write("SUCCESS", "Successful handshake");
                    }
                    OnHandshake?.Invoke();
                } else if (message.Code == TCPMessageCode.Ping) {

                    OnPingMessage(message);

                } else if (message.Code == TCPMessageCode.Message) {

                    OnMessage?.Invoke(message);

                }

            }

        }
    }

    protected void OnPingMessage(TCPMessage message) {

        using (IOStream stream = new IOStream(message.Content)) {

            double rtt = 0;
            bool error = stream.ReadDouble(out rtt);

            string dateStr = null;
            error = stream.ReadString(out dateStr);

            RTT = rtt;

            try {

                if(error) {
                    return;
                }

                DateTime sent = DateTime.Parse(dateStr, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind);

                Send(new TCPMessage() {
                    Code = TCPMessageCode.Pong,
                    Content = Encoding.UTF8.GetBytes(sent.ToString("O"))
                });

            } catch(Exception er) {
                Console.WriteLine("Error");
            }

        }

    }

    /// <summary>
    /// Initialize the handlers
    /// </summary>
    protected void InitHandlers() {

        OnMessage += (message) => {
            
        };

    }

    /// <summary>
    /// Get appropiate stream of socket
    /// </summary>
    /// <param name="client"></param>
    /// <returns></returns>
    protected Stream GetStream() {

        Stream stream = new NetworkStream(Socket);

        if (SSL == null) {

            return stream;

        }

        try {

            SslStream sslStream = new SslStream(stream, false);
            sslStream.AuthenticateAsClient(Address.ToString());

            return sslStream;

        } catch (Exception e) {

            return null;

        }

    }

    /// <summary>
    /// Init new Socket instance
    /// </summary>
    protected void InitSocket() {

        Socket = new Socket(AddressFamily.InterNetworkV6, SocketType.Stream, ProtocolType.Tcp);
        Socket.SetSocketOption(SocketOptionLevel.IPv6, SocketOptionName.IPv6Only, false);

    }

    /// <summary>
    /// Disconnectes from Server
    /// </summary>
    public void Disconnect() {

        if(Running) {

            Send(new TCPMessage() {
                Code = TCPMessageCode.Close,
                Content = new byte[2] { 0, 1 }
            });

            try {

                Socket.Shutdown(SocketShutdown.Both);
                Socket.Disconnect(true);

            } catch(Exception er) {

            }

        }

    }

    /// <summary>
    /// Connect to server
    /// </summary>
    public void Connect() {

        if (Logging) {
            Logger.Write("REGION", "Method [Connect]");
        }

        bool connected = false;

        while(!connected) {

            if(Logging) {
                Logger.Write("INFO", "Trying to connect...");
            }

            InitSocket();

            try {

                Socket.Connect(new IPEndPoint(Address, Port));
                
                Running = connected = true;

                ListenThread = new Thread(Listen);
                ListenThread.Start();

            } catch (Exception e) {

                if (Logging) {
                    Logger.Write("FAILED", "Failed to connect");
                }

                Running = connected = false;

                if (Logging) {
                    Logger.Write("INFO", "Sleep for " + ReconnectSleep + "ms");
                }

                Thread.Sleep(ReconnectSleep);

            }

        }

    }

}

Example Usage

class Program {

    static void Main(string[] args) {

        new Thread(() => {

            Random rand = new Random();

            TCPServer server = new TCPServer(54545, null, IPAddress.Any);
            server.Start();

            server.OnMessage += (cl, mess) => {

                using (IOStream stream = new IOStream(mess.Content)) {

                    float fl;
                    stream.ReadFloat(out fl);

                    Console.WriteLine("Received from Client: " + fl);

                }

                using (IOStream stream = new IOStream()) {

                    float numb = (float)rand.NextDouble() * 999;
                    Console.WriteLine("Server sending number: " + numb);

                    stream.WriteFloat(numb);

                    cl.Send(new TCPMessage() {
                        Content = stream.ToArray()
                    });

                }

            };

        }).Start();

        new Thread(() => {

            Random rand = new Random();

            TCPClient client = new TCPClient("127.0.0.1", 54545);
            client.Connect();

            client.OnHandshake += () => {

                using (IOStream stream = new IOStream()) {

                    float numb = (float)rand.NextDouble() * 999;
                    Console.WriteLine("Client sending number: " + numb);

                    stream.WriteFloat(numb);

                    client.Send(new TCPMessage() {
                        Content = stream.ToArray()
                    });

                }

            };

            client.OnMessage += (mes) => {

                using (IOStream stream = new IOStream(mes.Content)) {

                    float fl;
                    stream.ReadFloat(out fl);

                    Console.WriteLine("Received from Server: " + fl);

                }

                using (IOStream stream = new IOStream()) {

                    float numb = (float)rand.NextDouble() * 999;
                    Console.WriteLine("Client sending number: " + numb);

                    stream.WriteFloat(numb);

                    client.Send(new TCPMessage() {
                        Content = stream.ToArray()
                    });

                }

            };

        }).Start();

    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Few tips: 1) I would like to ecapsulate the Logger calls with severity enum and check of Logging inside. 2) TAP async/await and Task is preferrable instead manual managing threads. \$\endgroup\$
    – aepot
    Sep 23 '20 at 17:27
1
\$\begingroup\$

Properties

Seems too many properties public and if they need to be public should use the readonly interface. For example in the tcpserver there is a property called ClientsList that is a List. IT would seem abnormal that an outside class could clear the list or add/remove clients from the list. If the property needs to be public returning back IReadOnlyList or IEnumerable would be a better more protective manner. Same with ClientsDict that seems that is an internal working and shouldn't be a public property and more likely a private readonly field.

Events

Instead of making delegates should follow the standard event pattern of using EventHandler<> and passing in EventArgs. While you will need to make more EventArgs classes it will be easier for others coming onto your project or if wanting to add other libraries.

Exceptions

GetStream is just eating exceptions and returning null but the code doesn't look like it's specifically looking for null being bad. Also should only catch exceptions you can handle and not the generic Exception class.

Logging

I would recommend to use the Microsoft.Extensions.Logging.Abstractions for logging. Lots of logging application already adapt to the MS ILogger interface. Then can log different types. For example in catching an exception call the logger.LogError with the exception. Then having other logging for debugging or info. Then can configure what level you want to log. For example in dev mode probably want to log debug or higher but when release warning or higher.

TPL

MS has an Async Socket Server and Client those examples are using the old APM and now can use the TPL. For example can now just do var client = await socket.AcceptAsync() Instead of the BeginAccept and EndAccept calls.

For pinging on a timer I would either use Task.Delay or System.Threading.Timer. The more you can offload to the TPL the better.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.