patterncsharpMinor
Basic TCP server client application
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:
Client:
```
internal void CreateClient(object message)
{
try
{
_infrastructure_TcpServerAndClient.PeerClient_Connect();
_infrastructure_TcpServerAndClient.SendMessageViaStreamWriter(message);
_infrastructure_TcpServerAndClient.CloseConnection();
}
catch (Exception ex)
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
camelCasecasing.
- the
Thread.Sleep(500);does make no sense after accepting the socket. Why do you want the thread to sleep ?
SendMessageViaStreamWriteris badly named.SendMessagewould 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.