Question

I'm trying to understand the SRP but, whilst I understand the reasoning behind how to apply it, I'm not really seeing the benefit of doing so. Consider this example, taken from Robert Martin's SRP PDF:

interface IModem
{
    void Dial(string number);
    void Hangup();
    void Send(char c);
    char Recv();
}

He proposes separating this into two interfaces:

interface IModemConnection
{
    void Dial(string number);
    void Hangup();
}

interface IModemDataExchange
{
    void Send(char c);
    char Recv();
}

I've also been reading this article, which takes this one step further:

interface IModemConnection : IDisposable
{
    IModemDataExchange Dial(string number);
}

interface IModemDataExchange
{
    void Send(char c);
    char Recv();
}

At this point, I understand what is meant by functional (Send / Recv) and non-functional (Dial / Hangup) aspects, but I don't see the benefit of separating them in this example. Considering this basic implementation:

class ConcreteModem : IModemConnection
{
    public IModemDataExchange Dial(string number)
    {
        if (connection is successful)
        {
            return new ConcreteModemDataExchange();
        }

        return null;
    }

    public void Dispose()
    {
        // 
    }

    public bool IsConnected { get; private set; }
}

At this point, let me quote Robert Martin again, even though he's talking about a different example from that PDF:

Secondly, if a change to the GraphicalApplication causes the Rectangle to change for some reason, that change may force us to rebuild, retest, and redeploy the ComputationalGeometryApplication. If we forget to do this, that application may break in unpredictable ways.

This is what I don't understand. If I had to create a second implementation of IModemDataExchange, and I wanted to make use of that, I would still have to change the Dial method, meaning the class also needs to be recompiled:

public IModemDataExchange Dial(string number)
{
    if (some condition is met)
    {
        return new ConcreteModemDataExchange();
    }
    else if (another condition is met)
    {
        return new AnotherConcreteModemDataExchange();
    }

    return null;
}

I can't see what this has done to reduce the effects of change on the class. It still needs to be recompiled, so what's the benefit? What do you gain from doing this that is so important to producing quality code?

Was it helpful?

Solution

To me the modem example above always seemed like a case for the interface segregation principle rather than SRP, but that's besides the point.

In the part you called out regarding the Rectangle, I think you're just misinterpreting it. Martin is using the Rectangle as an example of a shared library. If the GraphicalApplication requires a new method or change in semantics in the Rectangle class, then that affects the ComputationalGeometryApplication since they both "link" to the Rectangle library. He's saying it violates SRP because it is responsible for defining rendering bounds and also an algebraic concept. Imagine if the GraphicalApplication changed from DirectX to OpenGL where the y-coordinate is inverted. You may want to change some things around on the Rectangle to facilitate this, but you're then potentially causing changes in ComputationalGeometryApplication.

In my work, I try to follow the SOLID principles and TDD, and I've found that SRP makes writing tests for classes simple and also keeps the classes focused. Classes that follow SRP are typically very small, and this reduces complexity both in code and dependencies. When stubbing out classes, I try to make sure a class is either "doing one thing", or "coordinating two (or more) things". This keeps them focused and makes their reasons for change only dependent on the one thing they do, which to me is the point of SRP.

OTHER TIPS

The main benefit is quite obvious. By splitting up you're giving your model better logical grouping which in turn makes the intention clearer and maintenance easier.

If I had to create a second implementation of IModemDataExchange, and I wanted to make use of that, I would still have to change the Dial method

Yes will have to, but that is not the benefit there. One benefit is, when you have any modification for IModemDataExchange interface itself you only have to change concrete implementations of the interface, not ConcreteModem itself which will make maintenance of subscribers of Dial method easier. Another benefit is that now even if you have to write an additional IModemDataExchange implementation, then changes it will require in ConcreteModem class is minimized, there isn't a direct coupling. By separating responsibilities you minimize the side effects of modifications.

Not requiring to recompile is not the essence here. And in a strict sense, what if one of those interface is in another project? It saves recompilation of one project. The stress is on not requiring to change code at lot of places. Of course any change would require a recompilation.

You don't need to change ConcreteModem if you use an abstract factory. Or if you parameterize generic Modem<TModemDataExchange> (or generic Dial<TModemDataExchange>() method) by a concrete type that should be created on success.

The idea is that IModemConnection implementation doesn't depend on any info about IModeDataExchange implementation except its name.

Moving forward, I'd consider following approach:

interface IModemConnection : IDisposable
{
    void Dial(string number);
}

interface IModemDataExchange
{
    void Send(char c);
    char Recv();
}

class ConcreteModemDataExchange : IModemDataExchange
{
    ConcreteModemDataExchange(IModemDataExchange);
}

So to create ConcreteModemDataExchange instance you need to have a connection. There's still a possibility to have disconnected instance of connection, but it's a different story.

As a side node, I'd recommend to throw an exception in Dial on failure.

I don't know a lot about how modems work, so I struggled a bit to come up with a meaningful example. However, consider this:

Having separated the logic for dialing, now if some other parts of the program only need to dial, we can pass in just a IModemConnection. This could be useful even in a Modem class itself, using dependency injection:

public class Modem : IModemConnection, IModemDataExchange
{
    public IModemConnection Dialer {get; private set;}

    public Modem(IModemConnection Dialer)
    {
        this.Dialer=Dialer;
    }

    public void Dial(string number)
    {
        Dialer.Dial(number);
    }

    public void Hangup()
    {
        Dialer.Hangup();
    } 

    // .... implement IModemDataExchange
}

Now you could have:

public class DigitalDialer : IModemConnection
{
    public void Dial(string number)
    {
        Console.WriteLine("beep beep");
    }
    public void Hangup()
    {
        //hangup
    }
}

and

public class AnalogDialer : IModemConnection
{
    public void Dial(string number)
    {
        Console.WriteLine("do you even remember these?");
    }
    public void Hangup()
    {
        //hangup
    }
}

now, if you want to change some aspects of how your modem works (the way it dials a number in this case) your changes will be localized in a Dialer class that has a single responsibility (dialing).

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top