Is it good practise to blank out inherited functionality that will not be used?

Question

I'm wondering if I should change the software architecture of one of my projects.

I'm developing software for a project where two sides (in fact a host and a device) use shared code. That helps because shared data, e.g. enums can be stored in one central place.

I'm working with what we call a "channel" to transfer data between device and host. Each channel has to be implemented on device and host side. We have different kinds of channels, ordinary ones and special channels which transfer measurement data.

My current solution has the shared code in an abstract base class. From there on code is split between the two sides. As it has turned out there are a few cases when we would have shared code but we can't share it, we have to implement it on each side.

The principle of DRY (don't repeat yourself) says that you shouldn't have code twice.

My thought was now to concatenate the functionality of e.g. the abstract measurement channel on the device side and the host side in an abstract class with shared code. That means though that once we create an actual class for either the device or the host side for that channel we have to hide the functionality that is used by the other side.

Is this an acceptable thing to do:

public abstract class ChannelAbstract
{
    protected void ChannelAbstractMethodUsedByDeviceSide()    {  }
    protected void ChannelAbstractMethodUsedByHostSide()      {  }
}

public abstract class MeasurementChannelAbstract : ChannelAbstract
{
    protected void MeasurementChannelAbstractMethodUsedByDeviceSide()   {  }
    protected void MeasurementChannelAbstractMethodUsedByHostSide()     {  }
}

public class DeviceMeasurementChannel : MeasurementChannelAbstract
{
    public new void MeasurementChannelAbstractMethodUsedByDeviceSide()
    {
        base.MeasurementChannelAbstractMethodUsedByDeviceSide();
    }

    public new void ChannelAbstractMethodUsedByDeviceSide()
    {
        base.ChannelAbstractMethodUsedByDeviceSide();
    }
}

public class HostMeasurementChannel : MeasurementChannelAbstract
{
    public new void MeasurementChannelAbstractMethodUsedByHostSide()
    {
        base.MeasurementChannelAbstractMethodUsedByHostSide();
    }

    public new void ChannelAbstractMethodUsedByHostSide()
    {
        base.ChannelAbstractMethodUsedByHostSide();
    }
}

Now, DeviceMeasurementChannel is only using the functionality for the device side from MeasurementChannelAbstract. By declaring all methods/members of MeasurementChannelAbstract protected you have to use the new keyword to enable that functionality to be accessed from the outside.

Is that acceptable or are there any pitfalls, caveats, etc. that could arise later when using the code?

Answer 1

It sounds to me as though you've not finished identifying which bits of code are shared. Wouldn't it make more sense to keep all the generic / shared stuff in MeasurementChannelAbstract rather than having different methods which the two sides call? I would have thought that those ought to be in inherited classes?

Answer 2

You can solve the problem with inheritance, like this:

public abstract class MeasurementChannelAbstract
{
    protected abstract void Method();
}

public class DeviceMeasurementChannel : MeasurementChannelAbstract
{
    public void Method()
    {
        // Device side implementation here.
    }
}

public class HostMeasurementChannel : MeasurementChannelAbstract
{
    public void Method()
    {
        // Host side implementation here.
    }
}

... or by composition, using the Strategy pattern, like this:

public class MeasurementChannel
{
    private MeasurementStrategyAbstract m_strategy;

    public MeasurementChannel(MeasurementStrategyAbstract strategy)
    {
        m_strategy = strategy;
    }

    protected void Method()
    {
        m_strategy.Measure();
    }
}

public abstract class MeasurementStrategyAbstract
{
    protected abstract void Measure();
}

public class DeviceMeasurementStrategy : MeasurementStrategyAbstract
{
    public void Measure()
    {
        // Device side implementation here.
    }
}

public class HostMeasurementStrategy : MeasurementStrategyAbstract
{
    public void Measure()
    {
        // Host side implementation here.
    }
}

It seems to me that you want to divide your inheritance hierarchy between both Standard/Measurement channels and Device/Host channels. One way to do this is with multiple inheritance - but C# doesn't support multiple inheritance (except for interfaces), and in most cases a design based on composition will be simpler.

Answer 3

To me it looks a bit like you're confusing inheritance and composition. When you have to "blank out"/throw exception when inherited functionality does not overlap sanely, your inheritance graph is missing some intermediate class. And often this is because some functionality just should come from a member instance of an other class instead of being inherited.

Consider also practicality, mapping everything into perfect OOP is not the goal, the goal is a working program that is maintainable without huge pain.

Answer 4

Like Rich implies: Needing only ONE of the members declared ins MeasurementChannelAbstract in one of the concrete implementations is a very clear indication that your interface is badly defined as it has more than one responsibility. This means it is difficult for clients of your code (and readers) to understand the abstractions and see the difference between the various concrete implementations.

This is called the "Single Responsibility Principle" and it is is an important one for good OO design.

(For more on good OO design, I recommend working through all the SOLID principles).

Answer 5

Therefore I ask myself if it is OK to have a single chain of inheritance (accepting that some classes will have functionality for both host and device side) and do the separation into host/device channel at the last stage (blanking out the functionality which is not needed)

I think it's ok. But you may blanking out the functionality which is not needed by using different interfaces for SimpleChannel and MeasurementChannel.