TCP client reconnection logic running on 30 machines I am working on a service that is supposed to run on a large installation 24/7. Once the software is deployed, I will most likely not be able to make any changes anymore. This is my first time working on a network application so I want to make sure I am not making any obvious mistakes.

60 instances of this program are supposed to run on 30 machines, meaning there are 2 instances of it running on each machine. It is used to keep all the client instances (running this code) connected to the controller that gives them updates when needed.

I am wondering if there are any optimizations you guys can recommend to me or if there are any other insufficiencies that catch your eyes.

public class ControllerConnectionService {

    public ControllerConnectionService() {
        Initialize();
    }

    private Timer _pingTimer;
    private SimpleTcpClient _tcpClient;

    public event EventHandler&lt;DataModel&gt; ContentTypeChanged;

    private void Initialize() {
        CreateTcpClient();
        Connect();

        var messageString = GetMessageString(MessageType.SendId, MainViewModel.Id.ToString());
        SendMessage(messageString);

        _pingTimer = new Timer {Interval = 2000};
        _pingTimer.Elapsed += PingTimerOnElapsed;
        _pingTimer.Start();
    }

    private void PingTimerOnElapsed(object sender, ElapsedEventArgs elapsedEventArgs) {
        if (!IsConnected()) Reconnect();
    }

    private bool IsConnected() {
        try {
            var pingBytes = Encoding.UTF8.GetBytes(GetMessageString(MessageType.Ping, string.Empty));
            _tcpClient.TcpClient.Client.Send(pingBytes);
            return true;
        }
        catch (Exception) {
            return false;
        }
    }

    private void CreateTcpClient() {
        _tcpClient = new SimpleTcpClient();
        _tcpClient.DataReceived += TcpClientOnDataReceived;
    }

    private void Connect() {
        try {
            _tcpClient.Connect(ControllerIp, ClientControllerPort);
        }
        catch (Exception) {
            Reconnect();
        }
    }


    private void Reconnect() {
        try {
            while (!IsConnected()) {
                Thread.Sleep(500);
                CreateTcpClient();

                _tcpClient.Connect(ControllerIp, ClientControllerPort);
            }
        }
        catch (Exception) {
            Reconnect();
        }
    }

    private void SendMessage(string message) {
        try {
            _tcpClient.TcpClient.Client.Send(Encoding.UTF8.GetBytes(message));
        }
        catch (Exception) {
            Reconnect();
        }
    }

    private void TcpClientOnDataReceived(object sender, Message message) {
        if (string.IsNullOrEmpty(message.MessageString))
            return;

        var messageParts = message.MessageString.Split(';');

        Enum.TryParse(messageParts[0], out MessageType messagetype);

        if (messagetype == MessageType.UpdateContentType) {
            Console.WriteLine($@"{DateStamp} Data received: {message.MessageString} ");
            var dataModel = JsonConvert.DeserializeObject&lt;DataModel&gt;(messageParts[1]);

            ContentTypeChanged?.Invoke(this, dataModel);
        }
    }
}


If you require any additional information or code, feel free to ask.
catch (Exception)
{
  Reconnect();
}



The same pattern can be found in almost every method. Are you not interested in what happened? No logging or anything? Just ignore the exception?




_pingTimer = new Timer { Interval = 2000 };
Thread.Sleep(500);



Hardcoded numbers? No settings? What if this won't work and you have to tune it? You'll have to update all instances!




Enum.TryParse(messageParts[0], out MessageType messagetype);



Why isn't this inside an if?


if (messagetype == MessageType.UpdateContentType)



This is the next line following the TryParse but this can go sideways if the UpdateContentType is the first value of the enum it might be returned as a default value and the flow will enter the if even if the massage part wasn't in the correct format.




private void TcpClientOnDataReceived(object sender, Message message)



You have try/catches for everything but this method. If something goes wrong here the application will crash.




  60 instances of this program are supposed to run on 30 machines, meaning there are 2 instances of it running on each machine. It is used to keep all the client instances (running this code) connected to the controller that gives them updates when needed.


This sounds like too much. Why can't the client check for updates by itself? 


  Once the software is deployed, I will most likely not be able to make any changes anymore.


If you are not able to update it later then you should start writing all kinds of unit and integration tests to make sure it's bullet-proof but even then there will still be a bug somewhere... Not being able to deploy new versions is a terrible plan.