Practical Ways to Implement the Single Responsibility Principle (SRP)

designdesign-patternsobject-orientedrefactoringsingle-responsibility

Simply what are the practical techniques people use to check if a class violates the single responsibility principle?

I know that a class should have only one reason to change, but that sentence is somewhat lacking a practical way to really implement that.

The only way I found is to use the sentence "The ……… should ……… itself." where the first space is the class name and the later is the method (responsibility) name.

However, sometimes it's hard to figure out if a responsibility really violates the SRP.

Are there more ways to check for the SRP?

Note:

The question is not about what the SRP means, but rather a practical methodology or a series of steps to check and implement the SRP.

UPDATE

Report class

I have added a sample class that clearly violates the SRP. It would be great if people could use it as an example to explain how they approach the single responsibility principle.

The example is from here.

Best Answer

The SRP states, in no uncertain terms, that a class should only ever have one reason to change.

Deconstructing the "report" class in the question, it has three methods:

printReport
getReportData
formatReport

Ignoring the redundant Report being used in every method, it is easy to see why this violates the SRP:

The term "print" implies some kind of UI, or an actual printer. This class therefore contains some amount of UI or presentation logic. A change to the UI requirements will necessitate a change to the Report class.
The term "data" implies a data structure of some kind, but doesn't really specify what (XML? JSON? CSV?). Regardless, if the "contents" of the report ever change, then so will this method. There is coupling to either a database or a domain.
formatReport is just a terrible name for a method in general, but I'd assume by looking at it that it once again has something to do with the UI, and probably a different aspect of the UI than printReport. So, another, unrelated reason to change.

So this one class is possibly coupled with a database, a screen/printer device, and some internal formatting logic for logs or file output or whatnot. By having all three functions in one class, you're multiplying the number of dependencies and tripling the probability that any dependency or requirement change will break this class (or something else that depends on it).

Part of the problem here is that you've picked a particularly thorny example. You should probably not have a class called Report, even if it only does one thing, because... what report? Aren't all "reports" completely different beasts, based on different data and different requirements? And isn't a report something that's already been formatted, either for screen or for print?

But, looking past that, and making up a hypothetical concrete name - let's call it IncomeStatement (one very common report) - a proper "SRPed" architecture would have three types:

IncomeStatement - the domain and/or model class that contains and/or computes the information that appears on formatted reports.
IncomeStatementPrinter, which would probably implement some standard interface like IPrintable<T>. Has one key method, Print(IncomeStatement), and maybe some other methods or properties for configuring print-specific settings.
IncomeStatementRenderer, which handles screen rendering and is very similar to the printer class.
You could also eventually add more feature-specific classes like IncomeStatementExporter / IExportable<TReport, TFormat>.

This is made significantly easier in modern languages with the introduction of generics and IoC containers. Most of your application code does not need to rely on the specific IncomeStatementPrinter class, it can use IPrintable<T> and thus operate on any kind of printable report, which gives you all of the perceived benefits of a Report base class with a print method and none of the usual SRP violations. The actual implementation need only be declared once, in the IoC container registration.

Some people, when confronted with the above design, respond with something like: "but this looks like procedural code, and the whole point of OOP was to get us -away- from the separation of data and behavior!" To which I say: wrong.

The IncomeStatement is not just "data", and the aforementioned mistake is what causes a lot of OOP folks to feel they are doing something wrong by creating such a "transparent" class and subsequently start jamming all kinds of unrelated functionality into the IncomeStatement (well, that and general laziness). This class may start out as just data but, over time, guaranteed, it will end up as more of a model.

For example, a real income statement has total revenues, total expenses, and net income lines. A properly-designed financial system will most likely not store these because they are not transactional data - in fact, they change based on the addition of new transactional data. However, the calculation of these lines is always going to be exactly the same, no matter whether you are printing, rendering, or exporting the report. So your IncomeStatement class is going to have a fair amount of behaviour to it in the form of getTotalRevenues(), getTotalExpenses(), and getNetIncome() methods, and probably several others. It is a genuine OOP-style object with its own behaviour, even if it doesn't really seem to "do" much.

But the format and print methods, they have nothing to do with the information itself. In fact, it's not too unlikely that you'll want to have several implementations of these methods, e.g. a detailed statement for management and a not-so-detailed statement for the shareholders. Separating these independent functions out into different classes gives you the ability to choose different implementations at runtime without the burden of a one-size-fits all print(bool includeDetails, bool includeSubtotals, bool includeTotals, int columnWidth, CompanyLetterhead letterhead, ...) method. Yuck!

Hopefully you can see where the above, massively-parameterized method goes wrong, and where the separate implementations go right; in the single-object case, every time you add a new wrinkle to the printing logic, you have to change your domain model (Tim in finance wants page numbers, but only on the internal report, can you add that?) as opposed to just adding a configuration property to one or two satellite classes instead.

Implementing the SRP properly is about managing dependencies. In a nutshell, if a class already does something useful, and you are considering adding another method that would introduce a new dependency (such as a UI, a printer, a network, a file, whatever), don't. Think about how you could add this functionality in a new class instead, and how you could make this new class fit into your overall architecture (it's pretty easy when you design around dependency injection). That is the general principle/process.

Side note: Like Robert, I patently reject the notion that an SRP-compliant class should have only one or two state variables. Such a thin wrapper could rarely be expected to do anything truly useful. So don't go overboard with this.

Related Solutions

Design Patterns – Is the Single Responsibility Principle Applicable to Functions?

One often missed detail of the Single Responsibility Principle is that the "reasons for change" are grouped by use-case actors (you can see a full explanation here).

So, in your example, the calculatePay method will need to be changed whenever new types of Employees are required. Since one type of employee may have nothing to do with another, it would be a violation of the principle if you keep them together, since the change would affect different user-groups (or use-case actors) in the system.

Now, about whether the principle applies to functions: Even if you have a violation in only one method, you are still changing a class for more than one reason, so it is still a violation of SRP.

SRP Violation – How to Avoid Violating the SRP in Caching Management Class

If this class really was as trivial as it appears to be, then there would be no need to worry about violating the SRP. So what if a 3-line function has 2 lines doing one thing, and another 1 line doing another thing? Yes, this trivial function violates the SRP, and so what? Who cares? The violation of the SRP starts becoming a problem when things get more complicated.

Your problem in this particular case most probably stems from the fact that the class is more complicated than the few lines that you have shown us.

Specifically, the problem most probably lies in the fact that this class not only manages the cache, but also probably contains the implementation of the GetFluffiesFromDb() method. So, the violation of the SRP is in the class, not in the few trivial methods that are shown in the code that you posted.

So, here is a suggestion on how to handle all kinds of cases that fall within this general category, with the help of the Decorator Pattern.

/// Provides Fluffies.
interface FluffiesProvider
{
    Fluffies GetFluffies();
}

/// Implements FluffiesProvider using a database.
class DatabaseFluffiesProvider : FluffiesProvider
{
    public override Fluffies GetFluffies()
    {
        ... load fluffies from DB ...
        (the entire implementation of "GetFluffiesFromDb()" goes here.)
    }
}

/// Decorates FluffiesProvider to add caching.
class CachingFluffiesProvider : FluffiesProvider
{
    private FluffiesProvider decoree;
    private DateTime m_NextReload = DateTime.MinValue;
    private Fluffies m_Cache;

    public CachingFluffiesProvider( FluffiesProvider decoree )
    {
        Assert( decoree != null );
        this.decoree = decoree;
    }

    public override Fluffies GetFluffies()
    {
        if( DateTime.Now >= m_NextReload ) 
        {
             m_Cache = decoree.GetFluffies();
             m_NextReload = DatTime.Now + TimeSpan.FromHours(1);
        }
        return m_Cache;
    }
}

and it is used as follows:

FluffiesProvider provider = new DatabaseFluffiesProvider();
provider = new CachingFluffiesProvider( provider );
...go ahead and use provider...

Note how CachingFluffiesProvider.GetFluffies() is not afraid to contain the code that does the time checking and updating, because that's trivial stuff. What this mechanism does is to address and handle the SRP at the system design level, where it matters, not at the level of tiny individual methods, where it does not matter anyway.

Best Answer

Related Solutions

Design Patterns – Is the Single Responsibility Principle Applicable to Functions?

SRP Violation – How to Avoid Violating the SRP in Caching Management Class

Related Topic