I've only been coding C# a few weeks and was just hoping for a little constructive criticism of a socket server I've been working on:using System;using System.Collections.Generic;using System.Linq;using System.Text;using System.Net.Sockets;using System.Net;namespace NetworkCommunication{    public class TCPSocketServer : IDisposable    {        private int portNumber;        private int connectionsLimit;        private Socket connectionSocket;        private List<StateObject> connectedClients = new List<StateObject>();        public event SocketConnectedHandler ClientConnected;        public delegate void SocketConnectedHandler(TCPSocketServer socketServer, SocketConnectArgs e);        public event SocketMessageReceivedHandler MessageReceived;        public delegate void SocketMessageReceivedHandler(TCPSocketServer socketServer, SocketMessageReceivedArgs e);        public event SocketClosedHandler ClientDisconnected;        public delegate void SocketClosedHandler(TCPSocketServer socketServer, SocketEventArgs e);        #region Constructors        public TCPSocketServer(int PortNumber) : this(PortNumber, 0) { }        public TCPSocketServer(int PortNumber, int ConnectionsLimit)        {            this.portNumber = PortNumber;            this.connectionsLimit = ConnectionsLimit;            startListening();        }        #endregion        #region Send Messages        public void SendMessage(string MessageToSend, int clientID)        {            try            {                byte[] byData = System.Text.Encoding.UTF8.GetBytes(MessageToSend + \0);                foreach (StateObject client in connectedClients)                {                    if (clientID == client.id)                    {                        // Send message on correct client                        if (client.socket.Connected)                        {                            client.socket.Send(byData);                        }                        break;                    }                }            }            catch (SocketException) { }        }        public void SendMessage(byte[] MessageToSend, int clientID)        {            try            {                foreach (StateObject client in connectedClients)                {                    if (clientID == client.id)                    {                        // Send message on correct client                        if (client.socket.Connected)                        {                            client.socket.Send(MessageToSend);                        }                        break;                    }                }            }            catch (SocketException) { }        }        #endregion        #region Connection and Listening        private void startListening()        {            try            {                // Create listening socket                connectionSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);                connectionSocket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);                IPEndPoint ipLocal = new IPEndPoint(IPAddress.Any, this.portNumber);                // Bind to local IP Address                connectionSocket.Bind(ipLocal);                // Start Listening                connectionSocket.Listen(1000);                // Creat callback to handle client connections                connectionSocket.BeginAccept(new AsyncCallback(onClientConnect), null);            }            catch (SocketException) { }        }        private void onClientConnect(IAsyncResult asyn)        {            try            {                // Create a new StateObject to hold the connected client                StateObject connectedClient = new StateObject();                connectedClient.socket = connectionSocket.EndAccept(asyn);                if (connectedClients.Count == 0)                {                    connectedClient.id = 1;                }                else                {                    connectedClient.id = connectedClients[connectedClients.Count - 1].id + 1;                }                connectedClients.Add(connectedClient);                // Check against limit                if (connectedClients.Count > connectionsLimit)                {                    // No connection event is sent so close socket silently                    closeSocketSilent(connectedClient.id);                    return;                }                // Dispatch Event                if (ClientConnected != null)                {                    SocketConnectArgs args = new SocketConnectArgs();                    args.ConnectedIP = IPAddress.Parse(((IPEndPoint)connectedClient.socket.RemoteEndPoint).Address.ToString());                    args.clientID = connectedClient.id;                    ClientConnected(this, args);                }                // Release connectionSocket to keep listening if limit is not reached                connectionSocket.BeginAccept(new AsyncCallback(onClientConnect), null);                // Allow connected client to receive data and designate a callback method                connectedClient.socket.BeginReceive(connectedClient.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(onReceivedClientData), connectedClient);            }            catch (SocketException) { }            catch (ObjectDisposedException) { }        }        private void onReceivedClientData(IAsyncResult asyn)        {            String content = String.Empty;            // Receive stateobject of the client that sent data            StateObject dataSender = (StateObject)asyn.AsyncState;            try            {                // Complete aysnc receive method and read data length                int bytesRead = dataSender.socket.EndReceive(asyn);                if (bytesRead > 0)                {                    // More data could be sent so append data received so far                    dataSender.sb.Append(Encoding.UTF8.GetString(dataSender.buffer, 0, bytesRead));                    content = dataSender.sb.ToString();                    if ((content.Length > 0) || (content.IndexOf() > -1))                    {                        String formattedMessage = String.Empty;                        formattedMessage += content.Replace(\0, );                        // Dispatch Event                        if (MessageReceived != null)                        {                            SocketMessageReceivedArgs args = new SocketMessageReceivedArgs();                            args.MessageContent = formattedMessage;                            args.clientID = dataSender.id;                            MessageReceived(this, args);                        }                        dataSender.sb.Length = 0;                    }                    try                    {                        dataSender.socket.BeginReceive(dataSender.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(this.onReceivedClientData), dataSender);                    }                    catch (SocketException) { }                }                else                {                    closeSocket(dataSender.id);                }            }            catch (SocketException) { }            catch (ObjectDisposedException) { }        }        #endregion        #region Socket Closing        public void closeSocket(int SocketID)        {            foreach (StateObject client in connectedClients.ToList())            {                try                {                    if (SocketID == client.id)                    {                        client.socket.Close();                        client.socket.Dispose();                        // Dispatch Event                        if (ClientDisconnected != null)                        {                            SocketEventArgs args = new SocketEventArgs();                            args.clientID = client.id;                            ClientDisconnected(this, args);                        }                        connectedClients.Remove(client);                        break;                    }                }                catch (SocketException) { }            }        }        // This does not dispatch an event, this task is to be used when rejecting connections past the limit.        // No connection event is sent so no disconnection event should be sent.        private void closeSocketSilent(int SocketID)        {            foreach (StateObject client in connectedClients.ToList())            {                if (SocketID == client.id)                {                    try                    {                        client.socket.Close();                        client.socket.Dispose();                        connectedClients.Remove(client);                        break;                    }                    catch (SocketException) { }                }            }        }        public void closeAllSockets()        {            foreach (StateObject client in connectedClients.ToList())            {                closeSocket(client.id);            }        }        #endregion        public void Dispose()        {            this.ClientConnected = null;            this.ClientDisconnected = null;            this.MessageReceived = null;            connectionSocket.Close();        }    }}Here's a new updated version with the help I was given kindly below.  I've added error handling and moved some parts around:using System;using System.Collections.Generic;using System.Linq;using System.Text;using System.Net.Sockets;using System.Net;namespace NetworkCommunication{    public class TCPSocketServer : IDisposable    {        private const int MaxLengthOfPendingConnectionsQueue = 1000;        private int portNumber;        private int connectionsLimit;        private Socket connectionSocket;        private Dictionary<int, StateObject> connectedClients = new Dictionary<int, StateObject>();        public event SocketConnectedHandler ClientConnected;        public delegate void SocketConnectedHandler(TCPSocketServer socketServer, SocketConnectArgs e);        public event SocketMessageReceivedHandler MessageReceived;        public delegate void SocketMessageReceivedHandler(TCPSocketServer socketServer, SocketMessageReceivedArgs e);        public event SocketClosedHandler ClientDisconnected;        public delegate void SocketClosedHandler(TCPSocketServer socketServer, SocketEventArgs e);        #region Constructor        public TCPSocketServer(int PortNumber, int ConnectionsLimit = 0)        {            // Validate Port Number            if (PortNumber > 0 && PortNumber < 65536)            {                this.portNumber = PortNumber;            }            else            {                throw new InvalidPortNumberException(Ports number must be in the 1-65535 range. Note: 256 and bellow are normally reserved.);            }            this.connectionsLimit = ConnectionsLimit;            startListening();        }        #endregion        private StateObject GetClient(int clientId)        {            StateObject client;            if (!connectedClients.TryGetValue(clientId, out client))            {                return null;            }            return client;        }        #region Send Messages        public void SendMessage(string MessageToSend, int clientID)        {            byte[] data = System.Text.Encoding.UTF8.GetBytes(MessageToSend + \0);            SendMessage(data, clientID);        }        public void SendMessage(byte[] MessageToSend, int clientID)        {            StateObject client = GetClient(clientID);            if (client != null)            {                try                {                    if (client.socket.Connected)                    {                        client.socket.Send(MessageToSend);                    }                }                catch (SocketException)                {                    // Close socket                    closeSocket(clientID);                }            }        }        #endregion        #region Connection and Listening        private void startListening()        {            try            {                // Create listening socket                connectionSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);                connectionSocket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);                IPEndPoint ipLocal = new IPEndPoint(IPAddress.Any, this.portNumber);                // Bind to local IP Address                connectionSocket.Bind(ipLocal);                // Start Listening                connectionSocket.Listen(MaxLengthOfPendingConnectionsQueue);                // Create callback to handle client connections                connectionSocket.BeginAccept(new AsyncCallback(onClientConnect), null);            }            catch (SocketException)             {                throw new SocketCannotListenException(Cannot listen on this socket. Fatal Error);            }        }        private void onClientConnect(IAsyncResult asyn)        {            // Create a new StateObject to hold the connected client            StateObject connectedClient = new StateObject();            try            {                connectedClient.socket = connectionSocket.EndAccept(asyn);                connectedClient.id = !connectedClients.Any() ? 1 : connectedClients.Keys.Max() + 1;                connectedClients.Add(connectedClient.id, connectedClient);                // Check against limit                if (connectedClients.Count > connectionsLimit && connectionsLimit != 0)                {                    // No connection event is sent so close connection quietly                    closeSocket(connectedClient.id, true);                    return;                }                // Allow connected client to receive data and designate a callback method                connectedClient.socket.BeginReceive(connectedClient.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(onReceivedClientData), connectedClient);            }            catch (Exception)            {                closeSocket(connectedClient.id, true);            }            finally            {                // Perfomed here to not get any exceptions on the main socket caught up in client connection errors.                ReleaseConnectionSocket();            }            // Dispatch Event at the end as any errors in socket dispatch silent diconnections            if (ClientConnected != null)            {                SocketConnectArgs args = new SocketConnectArgs()                {                    ConnectedIP = IPAddress.Parse(((IPEndPoint)connectedClient.socket.RemoteEndPoint).Address.ToString()),                clientID = connectedClient.id                };                ClientConnected(this, args);            }        }        private void ReleaseConnectionSocket()        {            try            {                // Release connectionSocket to keep listening                connectionSocket.BeginAccept(new AsyncCallback(onClientConnect), null);            }            catch (SocketException)            {                throw new SocketCannotListenException(Cannot listen on the main socket. Fatal Error);            }        }        private void onReceivedClientData(IAsyncResult asyn)        {            // Receive stateobject of the client that sent data            StateObject dataSender = (StateObject)asyn.AsyncState;            try            {                // Complete aysnc receive method and read data length                int bytesRead = dataSender.socket.EndReceive(asyn);                if (bytesRead > 0)                {                    // More data could be sent so append data received so far                    dataSender.sb.Append(Encoding.UTF8.GetString(dataSender.buffer, 0, bytesRead));                    String content = dataSender.sb.ToString();                    if (!string.IsNullOrEmpty(content))                    {                        String formattedMessage = content.Replace(\0, );                        // Dispatch Event                        if (MessageReceived != null)                        {                            SocketMessageReceivedArgs args = new SocketMessageReceivedArgs() {                            MessageContent = formattedMessage,                            clientID = dataSender.id                        };                            MessageReceived(this, args);                        }                        dataSender.sb.Clear();                    }                    try                    {                        dataSender.socket.BeginReceive(dataSender.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(this.onReceivedClientData), dataSender);                    }                    catch (SocketException) {                        closeSocket(dataSender.id);                    }                }                else                {                    closeSocket(dataSender.id);                }            }            catch (SocketException ex) {                // Socket closed at other end                if (ex.ErrorCode == 10054)                {                    closeSocket(dataSender.id);                }                else                {                    closeSocket(dataSender.id);                }            }        }        #endregion        #region Socket Closing        public void closeSocket(int SocketID)        {            closeSocket(SocketID, false);        }        // This does not dispatch an event, this task is to be used when rejecting connections past the limit.        // No connection event is sent so no disconnection event should be sent.        private void closeSocket(int SocketId, bool silent)        {            StateObject client = GetClient(SocketId);            if (client == null)            {                return;            }            try            {                client.socket.Close();                client.socket.Dispose();                if (!silent)                {                    // Dispatch event                    if (ClientDisconnected != null)                    {                        SocketEventArgs args = new SocketEventArgs()                        {                            clientID = client.id                        };                        ClientDisconnected(this, args);                    }                }            }            catch (SocketException)            {                // Socket is being removed anyway.            }            finally            {                connectedClients.Remove(client.id);            }        }        public void closeAllSockets()        {            var keys = connectedClients.Keys;            foreach (int key in keys)            {                var client = connectedClients[key];                closeSocket(client.id);            }        }        #endregion        public void Dispose()        {            this.ClientConnected = null;            this.ClientDisconnected = null;            this.MessageReceived = null;            connectionSocket.Close();        }    }}
TCP Socket Server
c#;networking;socket;tcp
I will not comment on the actual TCP functionality itself.I am not competent for that.Stopping broadcast - on purpose?When you are broadcasting to the clients in SendMessage(), it seems like you stop broadcasting if you get problems reaching one of the clients. Is this intended?I would move the try/catch inside the foreach, and around the if. Then, in the catch, I would either explicitly break; out of the foreach or leave a comment like // Don't care, ignore and proceed.Empty try-catchesAs @ChaosPandion suggested, I would not leave empty try-catches.I would either handle them, try to refactor the code as not to throw them, or leave a comment with a quick explanation.Succinct codeIf you are using C#4.0, I would write the default values like this:    #region Constructors    // This behaves the same as the other version with two constructors.    public TCPSocketServer(int PortNumber, int ConnectionsLimit = 0)    {Parameters - caseYou use camelCase for local variables, which agrees to what I think is the de facto convention.It seems you use PascalCase for parameters. Shouldn't that be camelCase instead, same as local variables?Hungarian notationI would avoid Hungarian notation.I have no real suggestion for byte[] byData though. Maybe just data? encodedData?Code duplicationCode duplication is recipe for disaster. You will update one of the code sections and not the other, and so on...So I would suggest that the first SendMessage just reuses the second:public void SendMessage(string MessageToSend, int clientID){    byte[] data = System.Text.Encoding.UTF8.GetBytes(MessageToSend + \0);    SendMessage(data, clientID);}Magic numbersI would extract this number to a constant somewhere, and name it appropriately. SocketListenTimeoutMillisec?// Start ListeningconnectionSocket.Listen(1000);Black magicPeople got dunked for much less than this. :)content.IndexOf() > -1Does it do anything, considering that the string has Length > 0? Context:content = dataSender.sb.ToString();if ((content.Length > 0) || (content.IndexOf() > -1))Succint codeString formattedMessage = String.Empty;formattedMessage += content.Replace(\0, );could beString formattedMessage = content.Replace(\0, );Var declarations closer to usageString content = String.Empty;could move next to content = dataSender.sb.ToString();, and even be integrated into that line:string content = dataSender.sb.ToString();Use StringBuilder better// instead of dataSender.sb.Length = 0;dataSender.sb.Clear();useless .ToList()    foreach (StateObject client in connectedClients.ToList())Linq makes code succintforeach (StateObject client in connectedClients.ToList()) {    if (SocketID == client.id) {can be replaced withStateObject client = connectedClients.Where( conClient => conClient.id == SocketID );if( client != null ) {Object constructor// Parentheses are optional, if empty.SocketMessageReceivedArgs args = new SocketMessageReceivedArgs() {    MessageContent = formattedMessage,    clientID = dataSender.id};And same for other similar property assignments immediately after the respective constructor.My proposal of cleaner codeusing System;using System.Collections.Generic;using System.Linq;using System.Net;using System.Net.Sockets;using System.Text;namespace NetworkCommunication{    class StateObject { }    class SocketConnectArgs { }    class SocketMessageReceivedArgs { }    class SocketEventArgs { }    public class TCPSocketServer : IDisposable    {        // Or whatever the name.        const int MaxLengthOfPendingConnectionsQueue = 1000;        private int portNumber;        private int connectionsLimit;        private Socket connectionSocket;        private Dictionary<int, StateObject> connectedClients = new Dictionary<int, StateObject>();        public event SocketConnectedHandler ClientConnected;        public delegate void SocketConnectedHandler(TCPSocketServer socketServer, SocketConnectArgs e);        public event SocketMessageReceivedHandler MessageReceived;        public delegate void SocketMessageReceivedHandler(TCPSocketServer socketServer, SocketMessageReceivedArgs e);        public event SocketClosedHandler ClientDisconnected;        public delegate void SocketClosedHandler(TCPSocketServer socketServer, SocketEventArgs e);        #region Constructors        public TCPSocketServer(int portNumber, int connectionsLimit = 0) {            this.portNumber = portNumber;            this.connectionsLimit = connectionsLimit;            startListening();        }        #endregion        private StateObject GetClient(int clientId) {            StateObject client;            if(!connectedClients.TryGetValue(clientId, out client)) {                return null;            }            return client;        }        #region Send Messages        public void SendMessage(string messageToSend, int clientID) {            byte[] data = System.Text.Encoding.UTF8.GetBytes(messageToSend + \0);            SendMessage(data, clientID);        }        public void SendMessage(byte[] messageToSend, int clientID) {            StateObject client = GetClient(clientID);            if (client != null) {                try {                    if (client.socket.Connected) {                        client.socket.Send(messageToSend);                    }                } catch (SocketException) {                    // TODO: sending failed; disconnect from client, or?                }            }        }        #endregion        #region Connection and Listening        private void startListening() {            try {                // Create listening socket                connectionSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);                connectionSocket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);                IPEndPoint ipLocal = new IPEndPoint(IPAddress.Any, this.portNumber);                // Bind to local IP Address                connectionSocket.Bind(ipLocal);                // Start Listening                connectionSocket.Listen(MaxLengthOfPendingConnectionsQueue);                // Creat callback to handle client connections                connectionSocket.BeginAccept(new AsyncCallback(onClientConnect), null);            } catch (SocketException) {                // TODO: if we fail to start listening, is it even ok to continue?                // Consider that some of the bootstrapping actions might not even have been done.                // Thus execution will likely crash on next step.            }        }        private void onClientConnect(IAsyncResult asyn) {            try {                // Create a new StateObject to hold the connected client                StateObject connectedClient = new StateObject() {                    socket = connectionSocket.EndAccept(asyn),                    id = !connectedClients.Any() ? 1 : connectedClients.Keys.Max() + 1                };                connectedClients.Add(connectedClient);                // TODO: consider if we can instead do this at the beginning of the method.                // Check against limit                if (connectedClients.Count > connectionsLimit) {                    // No connection event is sent so close socket silently                    closeSocket(connectedClient.id, true);                    return;                }                // Dispatch Event                if (ClientConnected != null) {                    SocketConnectArgs args = new SocketConnectArgs() {                        ConnectedIP = IPAddress.Parse(((IPEndPoint)connectedClient.socket.RemoteEndPoint).Address.ToString()),                        clientID = connectedClient.id                    };                    ClientConnected(this, args);                }                // Release connectionSocket to keep listening if limit is not reached                connectionSocket.BeginAccept(new AsyncCallback(onClientConnect), null);                // Allow connected client to receive data and designate a callback method                connectedClient.socket.BeginReceive(connectedClient.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(onReceivedClientData), connectedClient);            } catch (SocketException) {                // TODO: should we closeSocketSilent()? Or?            } catch (ObjectDisposedException) {                // TODO: should we closeSocketSilent()? Or?            }        }        private void onReceivedClientData(IAsyncResult asyn) {            // Receive stateobject of the client that sent data            StateObject dataSender = (StateObject)asyn.AsyncState;            try {                // Complete aysnc receive method and read data length                int bytesRead = dataSender.socket.EndReceive(asyn);                if (bytesRead > 0) {                    // More data could be sent so append data received so far                    dataSender.sb.Append(Encoding.UTF8.GetString(dataSender.buffer, 0, bytesRead));                    if ( dataSender.sb.Length != 0                        && MessageReceived != null                        ) {                        // TODO: is it possible that multiple messages are in the sb?                        // Consider whether it's necessary to replace with newline.                        dataSender.sb.Replace(\0, null); // Removes them.                        // Dispatch Event                        SocketMessageReceivedArgs args = new SocketMessageReceivedArgs();                        args.MessageContent = dataSender.sb.ToString();                        args.clientID = dataSender.id;                        MessageReceived(this, args);                        dataSender.sb.Clear();                    }                    try {                        dataSender.socket.BeginReceive(dataSender.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(this.onReceivedClientData), dataSender);                    } catch (SocketException) { }                } else {                    closeSocket(dataSender.id);                }            } catch (SocketException) {                // TODO: should we closeSocketSilent()? Or?            } catch (ObjectDisposedException) {                // TODO: should we closeSocketSilent()? Or?            }        }        #endregion        #region Socket Closing        public void closeSocket(int socketID) {            closeSocket(socketID, false);        }        /// <param name=silent>Whether to skip dispatching the disconnection event. Used to cancel the bootstrapping of the client-server connection.</param>        private void closeSocket(int socketID, bool silent) {            StateObject client = GetClient(socketID);            if (client == null) {                return;            }            try {                client.socket.Close();                client.socket.Dispose();                if(!silent) {                    // Dispatch Event                    if (ClientDisconnected != null) {                        SocketEventArgs args = new SocketEventArgs();                        args.clientID = client.id;                        ClientDisconnected(this, args);                    }                }                // Moved to finnaly block: connectedClients.Remove(client.id);            } catch (SocketException) {                // Don't care. Or?            } finally {                connectedClients.Remove(client.id);            }        }        public void closeAllSockets() {            var keys = connectedClients.Keys;            foreach( int key in keys ) {                var client = connectedClients[key];                closeSocket(client.id);            }        }        #endregion        public void Dispose() {            ClientConnected = null;            ClientDisconnected = null;            MessageReceived = null;            connectionSocket.Close();        }    }}PS: you owe me a beer. :)(This was fun to do, though!)edit: camelCase for parameters; reduced code duplication; Dictionary<int, StateObject> instead of List; more object constructors.edit2: StringBuilder methods as suggested by @pstrjds.