TCP chat application with both server and client logic in same application

Here is my simple chat program. I'm wondering whether it has an acceptable design (both object oriented design and network design).

If you want to run it, I can provide the solution folder.

Server

```
public class Server
{
private TcpListener _tcpListener;
private readonly int _packetSize = 64;

private int _clientCount;
private readonly int _maxClientCount;
private Dictionary _clients;
private readonly object _token = new object();

public Action OnDataReceive;

public bool Running { get; set; }

public int Port
{
get { return ((IPEndPoint)_tcpListener.Server.LocalEndPoint).Port; }
}

public Dictionary Clients
{
get { return _clients; }
}

public Server(int port, int maxClientCount)
{
_clientCount = 0;
_maxClientCount = maxClientCount;
_clients = new Dictionary(_maxClientCount);

try
{
_tcpListener = new TcpListener(IPAddress.Any, port);
}
catch (Exception e)
{
CommandLine.Write(e.Message);
}
}

public void StartListen()
{
try
{
Running = true;
_tcpListener.Start();
CommandLine.Write("Started listening at port " + Port + ".");
}
catch (Exception e)
{
CommandLine.Write(e.Message);
}

while (Running)
{
lock (_token)
{
if (_clientCount >= _maxClientCount) { continue; }
}

var newClient = _tcpListener.AcceptTcpClient();
AddClient(newClient);
}
}

public void StopListen()
{
try
{
Running = false;
_tcpListener.Stop();
CommandLine.Write("Stopped listening at port " + Port + ".");
}
catch (Exception e)
{
CommandLine.Write(e.Message);
}
}

public void Send(Client client, string

Grouping you member variables will make it easier for a reader. I prefer the order to be constants, set once, mutable.

If a value is known at compile time, it can be const instead of readonly. If a value is set and instance initialization and never changed, it should be readonly.

For example, all of the member variables in Server can be const or readonly. _clientCount can be replaced with _clients.Count. This has the added benefit of not needing to manually increment and decrement the count.

StartListen() has a spin loop in it. This is bad because it will cause the CPU to continually perform operations when it should just be silently waiting. You can use an EventWaitHandle pause the thread when no more clients can be added and trigger it when a client leaves. There are a number of specific implementations based on common patterns.

OnDataReceive assumes that an event listener has been registered, but there is no guarantee that one exists.

StackExchange Code Review Q#79274, answer score: 3