HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Basic TCP server client application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
applicationtcpclientserverbasic

Problem

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)

Solution


  • 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.

Code Snippets

byte[] StreamMessage = new byte[9632*2];
    try
    {
    socket = peerListener.AcceptSocket();

    Thread.Sleep(500);

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

        Thread.Sleep(500);

        MessageLength = socket.Receive(StreamMessage, 0, StreamMessage.Length, SocketFlags.None);
    }
internal void SendMessageViaStreamWriter(object message)
{
    string messageOut = (string)message + "\n";
    using (StreamWriter sw = new StreamWriter(peerClient.GetStream()))
    {
        sw.Write(messageOut);
        sw.Flush();
    }
}

Context

StackExchange Code Review Q#73719, answer score: 6

Revisions (0)

No revisions yet.