5
\$\begingroup\$

I wanted to try C# network programming and wrote this simple client/server application. The idea is - many clients can connect to server and request some data. As an example they request a set of points on sine-wave (required number of points and time span is up to each user).
The server then calculates required points and sends them to each user.

As this is my first program of that kind (and because I frankensteined it from 2 different examples) I guess there are definitely errors/smell here and I would be really grateful to hear them.

First - server setup:

class Program
{
    static void Main(string[] args)
    {
        IPHostEntry iph = Dns.GetHostEntry(Dns.GetHostName());
        IPAddress serverAddress = iph.AddressList[1];
        int server_Port = 1337;
        int maxConnections = 10;

        Listener listener = new Listener(serverAddress, server_Port);   // Setup server
        listener.StartListening(maxConnections);                        // Start server

        Console.Read();
    }
}


// Here we accept new connections
class Listener
{
    //This is the socket that will listen to any incoming connections
    public Socket _serverSocket { get; private set; }
    public int Port             { get; private set; }
    public int maxConnections   { get; private set; }
    public IPAddress ipAddress  { get; private set; }

    public Listener(IPAddress ServerIp, int ServerPort)
    {
        ipAddress = ServerIp;
        Port = ServerPort;

        _serverSocket = new Socket(ServerIp.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
    }


    // Here we start waiting for new client
    public void StartListening(int MaxConnections)
    {
        maxConnections = MaxConnections;
        try
        {
            Console.WriteLine("Server started at IP:" + ipAddress.ToString() + "; port:" + Port.ToString() + ";\n");

            _serverSocket.Bind(new IPEndPoint(ipAddress, Port));                            // Setup server at selected endpoint
            _serverSocket.Listen(MaxConnections);                                           // Limit maximum number of clients
            _serverSocket.BeginAccept(new AsyncCallback(AcceptCallback), _serverSocket);    // Actual waiting
        }
        catch (Exception ex)
        {
            throw new Exception("Server starting error" + ex);
        }
    }

    // Here we go after receiving connection request
    private void AcceptCallback(IAsyncResult ar)
    {
        try
        {
            Socket temp = (Socket)ar.AsyncState;                            // ??
            Socket acceptedSocket = temp.EndAccept(ar);                     // Get socket of new client 
            ClientController.AddNewClient(acceptedSocket);                  // Handle new client

            IPEndPoint REP = (IPEndPoint)acceptedSocket.RemoteEndPoint;
            Console.WriteLine("Received request from IP:" + REP.Address.ToString() + "; port:" + REP.Port.ToString() + ";");

            Console.WriteLine(ClientController.AllClients.Count() + " clients connected now");
            Console.WriteLine();


            // Resume waiting for new clients
            _serverSocket.BeginAccept(new AsyncCallback(AcceptCallback), _serverSocket);
        }
        catch (Exception ex)
        {
            throw new Exception("Server listening error" + ex);
        }
    }
}

Client class and Client collection:

// Client class
class Client
{
    public int Id                   { get; private set; }
    public Socket _clientSocket     { get; private set; }
    public ClientSender Sender      { get; private set; }
    public ClientReceiver Receive   { get; private set; }

    public Client(Socket socket, int id)
    {
        Sender = new ClientSender(socket, id);

        Receive = new ClientReceiver(socket, id);
        Receive.StartReceiving();

        _clientSocket = socket;
        Id = id;
    }


    // Handling client's request
    public void HandleRequest(string request)
    {
        string[] cmd = request.Split('_');

        // Here as an example I return points on sine wave based on user's request
        double tSpan;   double.TryParse(cmd[1], out tSpan);
        int nPoints;    int.TryParse(cmd[3], out nPoints);

        double tStep = tSpan / nPoints;

        for (int i = 0; i < nPoints; i++)
        {
            double ti = 0 + i * tStep;
            double val = 10 * Math.Sin(2 * Math.PI * ti);
            string DataToSend = "Точка (_" + ti.ToString() + "_,_" + val.ToString() + "_)";

            Sender.AnswerRequest(DataToSend);

            Thread.Sleep((int)(1000.0 * tStep));
        }
    }
}

// Class, which controlls all connected clients
static class ClientController
{
    // All connected clients in a list
    public static List<Client> AllClients = new List<Client>();


    // Handling new client (accepting/denying connection)
    public static void AddNewClient(Socket socket)
    {
        Client newClient = new Client(socket, AllClients.Count);
        AllClients.Add(newClient);
    }


    // Removing client
    public static void RemoveClient(int id)
    {
        int TargetClientIndex = AllClients.FindIndex(x => x.Id == id);
        AllClients.RemoveAt(TargetClientIndex);
    }


    // Serving client request (accepting/denying it)
    public static void AddClientRequest(int id, string data)
    {
        int TargetClientIndex = AllClients.FindIndex(x => x.Id == id);
        AllClients.ElementAt(TargetClientIndex).HandleRequest(data);
    }
}

And communications with clients:

// Class for receiving messages from client
public class ClientReceiver
{
    private byte[] _buffer;
    private Socket _receiveSocket;
    private int _clientId;

    public ClientReceiver(Socket receiveSocket, int Id)
    {
        _receiveSocket = receiveSocket;
        _clientId = Id;
    }


    // Start waiting for message from client
    public void StartReceiving()
    {
        try
        {
            _buffer = new byte[4];
            _receiveSocket.BeginReceive(_buffer, 0, _buffer.Length, 0, ReceiveCallback, null);
        }
        catch (Exception ex)
        {
            throw new Exception("Receiving start error" + ex);
        }
    }

    // Receiving message
    private void ReceiveCallback(IAsyncResult AR)
    {
        try
        {
            if (_receiveSocket.EndReceive(AR) > 1)
            {
                // First 4 bytes store the size of incoming messages - read them
                int MessageLength = BitConverter.ToInt32(_buffer, 0);

                // Knowing the full size of incoming message - prepare for receiving
                _buffer = new byte[MessageLength];

                // Receive
                _receiveSocket.Receive(_buffer, MessageLength, SocketFlags.None);
                string data = Encoding.Unicode.GetString(_buffer);

                Console.WriteLine("User " + _clientId.ToString() + " sent following request: " + data);

                // Send received message for handling
                ClientController.AddClientRequest(_clientId, data);

                // Resume waiting for new message 
                StartReceiving();
            }

            // if we didn't receive anything - disconnect client
            else
            {
                Disconnect();
            }
        }
        catch
        {
            if (!_receiveSocket.Connected)
            {
                Disconnect();
            }
            else
            {
                Console.WriteLine("Data receive error");
                StartReceiving();
            }
        }
    }

    // Disconnecting client
    private void Disconnect()
    {
        // Close connection
        _receiveSocket.Disconnect(true);
        ClientController.RemoveClient(_clientId);
    }
}


// Class, used to send messages back to selected client
class ClientSender
{
    private Socket _senderSocket;
    private int _clientId;

    public ClientSender(Socket receiveSocket, int Id)
    {
        _senderSocket = receiveSocket;
        _clientId = Id;
    }

    // Sending message to client
    public void AnswerRequest(string data)
    {
        try
        {
            byte[] DataPart = Encoding.Unicode.GetBytes(data);

            int SendMsgLength = DataPart.Length;
            byte[] InfoPart = BitConverter.GetBytes(SendMsgLength);

            var fullPacket = new List<byte>();
            fullPacket.AddRange(InfoPart);
            fullPacket.AddRange(DataPart);

            _senderSocket.Send(fullPacket.ToArray());
        }
        catch (Exception ex)
        {
            throw new Exception("Data sending error" + ex);
        }
    }

    // Disconnecting client
    private void Disconnect()
    {
        // Close connection
        _senderSocket.Disconnect(true);
        ClientController.RemoveClient(_clientId);
    }
}

On the client side: GUI part:

public delegate void UpdateCallback(string message);

public partial class MainWindow : Window
{
    public MainWindow()
    {
        InitializeComponent();
    }


    private void ConnectClick(object sender, EventArgs e)
    {
        IPHostEntry iph = Dns.GetHostEntry(Dns.GetHostName());
        IPAddress serverAddress = iph.AddressList[1];
        int server_Port = 1337;

        Connection.TryToConnect(serverAddress, server_Port);

        Connection.NewDataReceived += Foo_Changed;

        data_outp.Items.Add("Connection Succesfull");
    }

    private void SendClick(object sender, EventArgs e)
    {
        double tSpan;   double.TryParse(tSpan_input.Text, out tSpan);
        int nPoints;    int.TryParse(nPoints_input.Text, out nPoints);

        string DataToSend = "PLS GIMME THIS tSpan=_" + tSpan.ToString() + "_  nPoints=_" + nPoints.ToString();
        Connection.SendRequest(DataToSend);
    }


    private void Update(string message)
    {
        data_outp.Items.Add(message);
    }

    public void Foo_Changed(object sender, MyEventArgs args)  // the Handler (reacts)
    {
        data_outp.Dispatcher.Invoke(new UpdateCallback(Update), new object[] { args.Message });
    }

}

Interaction with server:

static class Connection
{
    public static Socket _connectingSocket { get; private set; }
    public static IPAddress ipAddress      { get; private set; }
    public static int Port                 { get; private set; }
    public static string ReceivedData      { get; private set; }
    private static byte[] _buffer;

    public static event EventHandler<MyEventArgs> NewDataReceived;

    // Trying connecting to selected server
    public static void TryToConnect(IPAddress ServerIp, int ServerPort)
    {
        ipAddress = ServerIp;
        Port = ServerPort;
        _connectingSocket = new Socket(ServerIp.AddressFamily, SocketType.Stream, ProtocolType.Tcp);

        while (!_connectingSocket.Connected)
        {
            Thread.Sleep(100);

            try
            {
                _connectingSocket.Connect(new IPEndPoint(ipAddress, Port));
                StartReceiving();
            }
            catch (Exception ex)
            {
                throw new Exception("Connection error" + ex);
            }
        }
    }


    // Start waiting for message from client
    public static void StartReceiving()
    {
        try
        {
            _buffer = new byte[4];
            _connectingSocket.BeginReceive(_buffer, 0, _buffer.Length, SocketFlags.None, ReceiveCallback, null);
        }
        catch (Exception ex)
        { 
            throw new Exception("Receiving start error" + ex);
        }
    }


    // Receiving message
    private static void ReceiveCallback(IAsyncResult AR)
    {
        try
        {
            if (_connectingSocket.EndReceive(AR) > 1)
            {
                // First 4 bytes store the size of incoming messages - read them
                int MessageLength = BitConverter.ToInt32(_buffer, 0);

                // Knowing the full size of incoming message - prepare for receiving
                _buffer = new byte[MessageLength];

                // Receive
                _connectingSocket.Receive(_buffer, _buffer.Length, SocketFlags.None);

                // Handle
                ReceivedData = Encoding.Unicode.GetString(_buffer, 0, MessageLength);
                if (ReceivedData.Length != 0)
                    NewDataReceived?.Invoke(null, new MyEventArgs(null, ReceivedData));

                // Resume waiting for new message 
                StartReceiving();
            }
            else
            {
                // Received nothing
            }
        }
        catch (Exception ex)
        {
            throw new Exception("Data receive error" + ex);
        }
    }


    // Send message to server
    public static void SendRequest(string DataToSend)
    {
        try
        {
            byte[] DataPart = Encoding.Unicode.GetBytes(DataToSend);

            int SendMsgLength = DataPart.Length;
            byte[] InfoPart = BitConverter.GetBytes(SendMsgLength);


            var fullPacket = new List<byte>();
            fullPacket.AddRange(InfoPart);
            fullPacket.AddRange(DataPart);

            _connectingSocket.Send(fullPacket.ToArray());

            Console.WriteLine("Sending request: " + DataToSend);
            Console.WriteLine("Infobytes length=" + InfoPart.Length + " bytes ; Total message length=" + SendMsgLength.ToString() + " bytes;");
        }
        catch (Exception ex)
        {
            throw new Exception("Data send error" + ex);
        }
    }
}

And an event to pass received data back to main GUI thread:

// My event to pass received message
public class MyEventArgs : EventArgs
{
    public MyEventArgs(Exception ex, string msg)
    {
        Error = ex;
        Message = msg;
    }

    public Exception Error { get; }

    public string Message { get; }
}

It does everything I need as of now (I can plot/save data and all that) but i guess there's room for improvement. Especially on the client side - I don't really like that event driven part, but couldn't make true async data pass.

\$\endgroup\$
1
\$\begingroup\$

ClientController

  • I don't see a purpose for this class. I would move AllClients, AddNewClient, RemoveClient to Listener and AddClientRequest to Client.
  • These operations should be made thread-safe.

Client

  • Don't start an async operation int the constructor. Create a method Initialise() and let this method call Receive.StartReceiving().

ClientReceiver

  • ReceiveCallback expects _receiveSocket.Receive to contain one message only and the full message. This should not be asserted. The underying socket is optimized to use a buffer for sending data. You should be able to deal with parts of messages and multiple messages. Accomodating this adds some complexity though, you should:

    • Use a raw buffer queue per client
    • Create a lexer/parser per client to determine when a full message is available in the queue

Common Guidelines

  • Use camelCase for arguments

    public Listener(IPAddress ServerIp, int ServerPort)

    public Listener(IPAddress serverIp, int serverPort)

  • Guard arguments

     public Listener(IPAddress serverIp, int serverPort)
     {
       // ..
     }
    

    public Listener(IPAddress serverIp, int serverPort) { if (serverIp == null) throw new ArgumentNullException(nameof(serverIp)); // .. }

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.