Simple TCP Send Receive with multithreading c#

Question

Wondering how this sample program can be made better. Specifically what exceptions should I expect to handle? Right now it's working as expected. I put a breakpoint after the statement:

int numBytesReadFromStream = stream.Read(bytes, 0, bytes.Length);

to check to see if the bytes variable had data in it and it did.

using System.Threading;
using System.Net;
using System.Net.Sockets;
using System;

namespace ConsoleApp2
{
    class Program
    {
        public static void Main()
        {
            var localIP = IPAddress.Parse("10.10.10.117");
            var listenerPort = 20000;
            var senderPort = listenerPort + 2;
            IPEndPoint listenerEndpoint = new IPEndPoint(localIP, listenerPort);
            IPEndPoint senderEndpoint = new IPEndPoint(localIP, senderPort);
            TcpListener tcpListener = new TcpListener(listenerEndpoint);
            StarttcpListenerThread(tcpListener);
            Thread.Sleep(300);
            StartTCPSendingThread(listenerEndpoint, senderEndpoint);
            while (true)
            {
            }
        }

        static void StarttcpListenerThread(TcpListener tcpListener)
        {
            tcpListener.Start();
            Thread tcpListenerThread = new Thread(() =>
            {
                while (true)
                {
                    try
                    {
                        byte[] bytes = new byte[1024];
                        TcpClient currentConnection = tcpListener.AcceptTcpClient();
                        NetworkStream stream = currentConnection.GetStream();
                        int numBytesReadFromStream = stream.Read(bytes, 0, bytes.Length);
                    }
                    catch (Exception e)
                    {
                        Console.WriteLine(e.Message);
                    }

                }
            });
            tcpListenerThread.Start();
        }

        static void StartTCPSendingThread(IPEndPoint listenerEndpoint, IPEndPoint senderEndpoint)
        {
            TcpClient tcpClient = new TcpClient(senderEndpoint);
            tcpClient.Connect(listenerEndpoint);
            Thread tcpSendingThread = new Thread(() =>
            {
                while (true)
                {
                    try
                    {
                        tcpClient.Client.Send(new byte[] { 1, 2, 8 });
                    }
                    catch (Exception e)
                    {
                        Console.WriteLine(e.Message);
                    }
                }
            });
            tcpSendingThread.Start();
        }
    }
}

Answer 1

There are several things that can be improved on the current code.

Starting by the listener, if you want to create a TcpListener you can use

var tcpListener = new TcpListener(IPAddress.Loopback, 20000); or 
var tcpListener = new TcpListener(IPAddress.Any, 20000);

The Main method looks a bit like spaghetti code. Specially because you keep alternating statements between things you need for the TcpListener and things that you need for TcpClient.

One possible thing you could do about this is to let StartTcpListenerThread create the TcpListener himself. Same goes for StartTcpSendingThread.

var senderPort = listenerPort + 2;

You don't have to be explicit about the port the client should use, since the operating system will attribute a port dynamically as long as you use TcpClient tcpClient = new TcpClient("localhost", 20000);

while (true) { }

This is great, if you want to increase your power bill by putting your CPU usage on 100. If you don't want to do that however, you can use Console.ReadLine(); or Thread.Sleep(Timeout.Infinite);

If possible you should also be consistent on whatever you use var or not.

Here is the code with the things I stated applied to it.

public static readonly int Port = 20000;

public static void Main()
{
    StartTcpListenerThread();
    Thread.Sleep(300);
    StartTcpSendingThread();
    Console.ReadLine();
}

static void StartTcpListenerThread()
{
    var tcpListener = new TcpListener(IPAddress.Any, Port);
    tcpListener.Start();
    Thread tcpListenerThread = new Thread(() =>
    {
        while (true)
        {
            try
            {
                var bytes = new byte[1024];
                var currentConnection = tcpListener.AcceptTcpClient();
                var stream = currentConnection.GetStream();
                var numBytesReadFromStream = stream.Read(bytes, 0, bytes.Length);
            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
            }

        }
    });
    tcpListenerThread.Start();
}

static void StartTcpSendingThread()
{
    var tcpSendingThread = new Thread(() =>
    {
        var tcpClient = new TcpClient("localhost", Port);
        while (true)
        {
            try
            {
                tcpClient.Client.Send(new byte[] { 1, 2, 8 });
            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
            }
        }
    });
    tcpSendingThread.Start();
}

Regarding your question about exceptions it seems reasonable to me, to catch the exception and display what is wrong to the console. Most of the time this will be due to some network issue and there isn't much you can do on those scenarios. Keeping the listener listening requests may or may not be a good decision but I would say it's at least a somewhat decent decision for a first approach.