How do you make inherited instance variables clear?

Question

I've been working with some abstract classes and something feels off but I'm not sure what to do about it. Here's a simple example to explain my question:

public abstract class Data
{
    protected Boolean _Connected = false;

    public abstract void Connect();
    public abstract void Disconnect();

    public Boolean Connected
    {
        get { return _Connected; }
    }

}

public class SqlServerData : Data
{
    public override void Connect()
    {
        // code to connect to sql server
        _Connected = true;
    }
    public override void Disconnect()
    {
        if (_Connected)
        {
            // code to disconnect from sql server
            _Connected = false;
        }
    }    
}

I have a couple problems with the design.

1. Nothing in this design forces a new implementation of Data to do anything with the _connected instance variable. If other classes using the Data class rely on the Connected property, it may not be accurate.

2. It can be confusing to someone new to this code. This sample is pretty simple but if there are more instance variables in the abstract class and other instance variables in the SqlServerData class it will be more difficult to know where the variables are defined.

So my question is, should this be refactored and if so, how?

Answer 1

Implement the state machine (connected or not connected, and the transitions between these states) in the base class, so the derived class doesn't have to worry about this part. Provide protected methods for the actual functionality, so the derived class can easily fill in the "holes":

public abstract class Data
{
    private bool connected;

    public bool Connected
    {
        get { return connected; }
    }

    public void Connect()
    {
        OnConnect();
        connected = true;
    }

    public void Disconnect()
    {
        if (connected)
        {
            OnDisconnect();
            connected = false;
        }
    }

    protected virtual void OnConnect() { }

    protected virtual void OnDisconnect() { }
}

and

public class SqlServerData : Data
{
    protected override void OnConnect()
    {
        // code to connect to sql server
    }

    protected override void OnDisconnect()
    {
        // code to disconnect from sql server
    }    
}

Answer 2

The problem is that you want the logic to manage the _connected flag to be shared amongst all subclasses, but still retain the ability for each subclass to provide its own implementation of what happens when you actually call Connect() and Disconnect(). For this I would recommend what's called a Template Method Pattern, so that the abstract base class can still retain some control over the logic used in the method. The way it works is pretty simple, just structure your class similar to this:

public void Connect() {
   if(!_connected) {
      ExecuteConnect();
      _connected = true;
   }
}

public void Disconnect() { 
   if(_connected) {
      ExecuteDisconnect();
      _connected = false;
   }
 }

 protected abstract void ExecuteConnect();
 protected abstract void ExecuteDisconnect();

Now in your base classes you can override the Execute* methods (I'm sure the naming convention can be improved) so that you can provide new implementations to Connecting and Disconnecting, while still managing control over the general algorithm.

Answer 3

I would rather make the property Connected abstract and ommit the protected field.

The disadvantage of protected fields is that the base and the children have to use it in a consistent way, thus coupling them stronger together.

But having the protected field might save you some simple lines of code. It depends on the actual problem, but generally I would rather write a few more lines of code then introduce coupling.