# Basic TCP server client application

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.Start();
int MessageLength = 0;
//_infrastructure_TcpServerAndClient.PeerListener_start();
Socket socket = null;
while (true)
{
if (peerListener.Pending())
{
continue;
}
else
{
byte[] StreamMessage = new byte[9632*2];
try
{
socket = peerListener.AcceptSocket();

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");
}

//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();

}
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?

• dead code should be removed
• the indention is horrible

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

MessageLength = socket.Receive(StreamMessage, 0, StreamMessage.Length, SocketFlags.None);
}


should be look like

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

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.
• THank you for your constructive answer ! point 1 : you right point 2 : what do you mean ? point 3 : can you please show me what do you mean ? – LordTitiKaka Dec 15 '14 at 16:05
• @Heslacher, I think there was a paste error. Only the first line of each method looks jacked up. – RubberDuck Dec 15 '14 at 16:10
• @RubberDuck, as all posted code is up for review ... and that isn't the only one, like you see in my updated answer. – Heslacher Dec 15 '14 at 16:13
• ahhhh my apologies @Heslacher I had missed that. Good catch. – RubberDuck Dec 15 '14 at 16:25
• @Heslacher , Great :) I'm going to mark this answer ofcourse But would you please give me some advise on the logic as well ? – LordTitiKaka Dec 16 '14 at 7:40