TcpClient field of abstract base class constantly being disposed

Question

Hi,

I have an abstract base class with a TcpClient field:

public abstract class ControllerBase
{
    internal protected TcpClient tcpClient;

It has a method to setup a connection:

private void setupConnection(IPAddress EthernetAddress, ushort TcpPort)
    {
        if (this.tcpClient == null || !this.tcpClient.Connected)
        {
            this.tcpClient = new TcpClient();

            try
            {
                this.tcpClient.Connect(EthernetAddress, TcpPort);
            }
            catch(Exception ex)
            {
                throw new TimeoutException("The device did not respond.\n" + ex.Message);
            }
        }
    }

And than methods to request data:

 internal protected virtual byte[] requestData(IPAddress EthernetAddress, ushort TcpPort, byte[] data, bool IgnoreResponse)
    {
        setupConnection(EthernetAddress, TcpPort);

        //The rest of the code uses this.tcpClient

There are a few others, such as requestRawData, etc... they are required for very specific hardware communication protocols, but that's not part of this question in any way.

I then have classes that derive from this class and they override the base class methods:

public class Controller : ControllerBase
{
  internal virtual byte[] requestData(byte[] data, bool IgnoreResponse)
  {
        return base.requestData(this.eth0.EthernetAddress, this.eth0.TcpPort, data, IgnoreResponse);
  }

The code works without any exceptions, but everytime the setupConnection method is called, the TcpClient instance (tcpClient) seems to be disposed, so a new one is created and the connect method is called again, really slowing down the communication process.

Note: Public methods of the child class call the requestData method, abstracting many details from the developer using this library.

Such as SetDevicePower(byte PowerLevel), QueryDeviceName() etc...

Code such as this:

Controller controller = new Controller("172.17.0.3",34000);
string name = controller.QueryDeviceName();
controller.SetDevicePower(200);

causes the connect method to be called twice... why is it being disposed between calls?

Answer 1

Hey TimothyP,

There are some inefficiencies in the "setupConnection" method that you may want to look into. The first concern is that you are instantiating the TcpClient when it's closed. This is not necessary. I would split the null check and connect logic into 2 methods, or at least two code blocks within the method:

  if (this.tcpClient == null)
  {
    this.tcpClient = new TcpClient();
  }

  try
  {
    if (!this.tcpClient.Connected)
    {
      this.tcpClient.Connect(EthernetAddress, TcpPort);
    }
  }
  catch(Exception ex)
  {
    throw new TimeoutException("The device did not respond.\n" + ex.Message);
  }

Secondly, the catch(Exception) is also a bad idea, You cannot assume the exception is a timeout, as there are numerous other exceptions that should be caught here.

As for your answer: you may have to provide further implementation details within your requestData method, as there could be a clue in there. For example, are you closing the connection? If so, you'd end up creating a new TcpClient object on the next call to setupConnection, which might be what's happening here.

Hope this sheds some light.