6
\$\begingroup\$

I want to build a robust and highly scalable client server system. Here what I have so far(an echo server as my base of implementation)

My Server

    private void startServer_Click(object sender, RoutedEventArgs e)
    {
        if (anyIP.IsChecked == true)
        {
            listener = new TcpListener(IPAddress.Any, Int32.Parse(serverPort.Text));
            Logger.Info("Ip Address : " + IPAddress.Any + " Port : " + serverPort.Text);
        }
        else
        {
            listener = new TcpListener(IPAddress.Parse(serverIP.Text), Int32.Parse(serverPort.Text));
            Logger.Info("Ip Address : " + serverIP.Text + " Port : " + serverPort.Text);
        }
        try
        {
            listener.Start();
            Logger.Info("Listening");
            HandleConnectionAsync(listener, cts.Token);
        }
        //finally
        //{
            //cts.Cancel();
            //listener.Stop();
            //Logger.Info("Stop listening");
        //}

        //cts.Cancel();
    }

    async Task HandleConnectionAsync(TcpListener listener, CancellationToken ct)
    {
        while (!ct.IsCancellationRequested)
        {
            Logger.Info("Accepting client");
            //TcpClient client = await listener.AcceptTcpClientAsync();
            TcpClient client = await listener.AcceptTcpClientAsync();
            Logger.Info("Client accepted");
            EchoAsync(client, ct);
        }

    }

    async Task EchoAsync(TcpClient client, CancellationToken ct)
    {
        var buf = new byte[4096];
        var stream = client.GetStream();
        while (!ct.IsCancellationRequested)
        {
            var amountRead = await stream.ReadAsync(buf, 0, buf.Length, ct);
            Logger.Info("Receive " + stream.ToString());
            if (amountRead == 0) break; //end of stream.
            await stream.WriteAsync(buf, 0, amountRead, ct);
            Logger.Info("Echo to client");
        }
    }

    private void stopServer_Click(object sender, RoutedEventArgs e)
    {
        cts.Cancel();
        listener.Stop();
        Logger.Info("Stop listening");
    }

My Client

    private void connect_Click(object sender, System.Windows.RoutedEventArgs e)
    {
        IPAddress ipAddress;
        int port;

        //TODO Check if ip address is valid
        ipAddress = IPAddress.Parse(serverIP.Text);
        //TODO port range is 0-65000
        port = int.Parse(serverPort.Text);

        StartClient(ipAddress, port);
    }

    private static async void StartClient(IPAddress serverIpAddress, int port)
    {
        var client = new TcpClient();
        //can i try/catch to catch await exception?
        try
        {
            await client.ConnectAsync(serverIpAddress, port);
        }
        catch (Exception e)
        {
            Logger.Info(e);                
        }
        Logger.Info("Connected to server");
        using (var networkStream = client.GetStream())
        using (var writer = new StreamWriter(networkStream))
        using (var reader = new StreamReader(networkStream))
        {
            writer.AutoFlush = true;
            for (int i = 0; i < 10; i++)
            {
                Logger.Info("Writing to server");
                await writer.WriteLineAsync(DateTime.Now.ToLongDateString());
                Logger.Info("Reading from server");
                var dataFromServer = await reader.ReadLineAsync();
                if (!string.IsNullOrEmpty(dataFromServer))
                {
                    Logger.Info(dataFromServer);
                }

            }
        }
        if (client != null)
        {
            client.Close();
            Logger.Info("Connection closed");
        }

    }
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$
  1. The general issue with your code is that your client and server implementations reside in your control's code-behind. It is always a good idea to separate business logic from UI.
  2. This is copy-paste:

    if (anyIP.IsChecked == true)
    {
        listener = new TcpListener(IPAddress.Any, Int32.Parse(serverPort.Text));
        Logger.Info("Ip Address : " + IPAddress.Any + " Port : " + serverPort.Text);
    }
    else
    {
        listener = new TcpListener(IPAddress.Parse(serverIP.Text), Int32.Parse(serverPort.Text));
        Logger.Info("Ip Address : " + serverIP.Text + " Port : " + serverPort.Text);
    }
    

    can be refactored to:

    var ip = anyIP.IsChecked == true ? IPAddress.Any : IPAddress.Parse(serverIP.Text);
    var port = Int32.Parse(serverPort.Text);
    listener = new TcpListener(ip, port);
    Logger.Info(String.Format("Ip Address : {0} Port : {1}", ip, port));
    
  3. Its a good style to prefix fields with underscore, so they can be distinguished from local variables. So it should be _listener instead of listener.

  4. You should shut down your server on exit.

  5. Yes, as long, as you await the call you can catch exceptions.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you. 1 and 2. I write it as fast as i can and i know its ugly. 3. I didnt know that. Thanks. 4. Youre right. 5. Thank you for that confirmation. \$\endgroup\$ Nov 5, 2013 at 7:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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