Basic TCP server client application

Question

I've written a basic server/client application to use in an automation application written in C#. The code is working pretty good, but I have a few thing I want to improve:

Server:

   public void socketListener()
{

    peerListener = new TcpListener(IPAddress.Any, ControlLayer.control_GlobalParam.TCP_PORT);
    peerListener.Start();
    int MessageLength = 0;
    //_infrastructure_TcpServerAndClient.PeerListener_start();
    Socket socket = null;
    while (true)
    {
        if (peerListener.Pending())
        {
            Thread.Sleep(500);
            continue;
        }
        else
        {
            byte[] StreamMessage = new byte[9632*2];
            try
            {
            socket = peerListener.AcceptSocket();

            Thread.Sleep(500);

                MessageLength = socket.Receive(StreamMessage, 0, StreamMessage.Length, SocketFlags.None);
            }
            catch (Exception ex)
            {
                //remote disconnected garcefully ? 
                Console.WriteLine(ex);

            }
            if (MessageLength > 0 )
            {
                string message = System.Text.Encoding.Default.GetString(StreamMessage);

                if (Domain_GlobalParam.IsCollectTcpServerOutput)
                {
                    onDataRecieved(this, "Collecting log from Remote station", "TcpServer");
                }
                else
                {
                    onDataRecieved(this, message, "TcpServer"); 
                }
                new Thread(() => ParseMessage(message)).Start();

                //ParseMessage(message);
            }
        }
    }
}

Client:

        internal void CreateClient(object message)
    {
        try
        {

            _infrastructure_TcpServerAndClient.PeerClient_Connect();
            _infrastructure_TcpServerAndClient.SendMessageViaStreamWriter(message);
            _infrastructure_TcpServerAndClient.CloseConnection();

        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.ToString());
        }
    }
    internal void PeerClient_Connect()
{
    peerClient = new TcpClient();
    peerClient.Connect(ComSettings.IpAddress, ComSettings.Port);

}
internal void SendMessageViaStreamWriter(object message)
{
    string MessageOut = (string)message + "\n";
    netStream = peerClient.GetStream();
    StreamWriter sw = new StreamWriter(netStream);
    sw.Write(MessageOut);
    sw.Flush();
}
internal void CloseConnection()
{
    netStream.Close();
    peerClient.Close();
}

I've noticed that from time to time I miss out some messages (especially long ones). What do you think I can improve here?

Answer 1

• dead code should be removed

• the indention is horrible

      byte[] StreamMessage = new byte[9632*2];
      try
      {
      socket = peerListener.AcceptSocket();
  
      Thread.Sleep(500);
  
          MessageLength = socket.Receive(StreamMessage, 0, StreamMessage.Length, SocketFlags.None);
      }  
  

  should be look like

      byte[] StreamMessage = new byte[9632*2];
      try
      {
          socket = peerListener.AcceptSocket();
  
          Thread.Sleep(500);
  
          MessageLength = socket.Receive(StreamMessage, 0, StreamMessage.Length, SocketFlags.None);
      }
  

• you should enclose IDisposable's like Stream, StreamWriter in using blocks.

  internal void SendMessageViaStreamWriter(object message)
  {
      string messageOut = (string)message + "\n";
      using (StreamWriter sw = new StreamWriter(peerClient.GetStream()))
      {
          sw.Write(messageOut);
          sw.Flush();
      }
  }
  

  The use of the using statement ensures that the Dispose() method is called in any case (like an exception has happened) this also ensures that the stream is closed.

• methods should be named using PascalCase casing and should be made out of verbs or verb phrases. See: Naming Guidelines

• local variables should be named using camelCase casing.
• the Thread.Sleep(500); does make no sense after accepting the socket. Why do you want the thread to sleep ?
• SendMessageViaStreamWriter is badly named. SendMessage would be much better. What would happen if you decide to change the implementation of the method but forget to change the name, ? Mr.Maintainer would be confused.