Do one thing

Saturday 28 April 2018This is over six years old. Be careful.

A famous and popular set of ideas for object-oriented programs are the SOLID design principles. The name is an acronym of the five principles:

These are all good ideas, but I struggle with the first one. The Single Responsibility Principle says that a class should have only one responsibility. But that’s difficult to be precise about. Doesn’t a class that has three methods do three things? How do you decide if it’s “one responsibility”?

To underscore my uncertainty, it’s easy to find seemingly contradictory examples written by people trying to explain the principle. In SOLID Design Principles Explained – The Single Responsibility Principle, Thorben Janssen says,

The [JPA] specification defines lots of different interfaces for [JPA], specifies a set of entity lifecycle states and the transitions between them, and even provides a query language, called JPQL.

But that is the only responsibility of the JPA specification.

He lists three things, and then says (singular) “that is the only responsibility”? How did we decide that all of that stuff is just one responsibility?

Another example: the SOLID principles were first enumerated by Robert C. Martin. In his book Clean Code, he applies the idea to functions as well as classes. He shows an example of an over-crowded function, and then says,

This bit of code does three things. It loops over all the employees, checks to see whether each employee ought to be paid, and then pays the employee. This code would be better written as:

public void pay() {
    for (Employee e : employees)
        payIfNecessary(e);
}
private void payIfNecessary(Employee e) {
    if (e.isPayday())
        calculateAndDeliverPay(e);
}
private void calculateAndDeliverPay(Employee e) {
    Money pay = e.calculatePay();
    e.deliverPay(pay);
}

Each of these functions does one thing.

That last function does one thing? Then why does it have the word “And” right in the name??

Don’t get me wrong. I’m not saying the Single Responsibility Principle is a bad idea. It’s a great idea. It’s just not quantifiable or precise. It’s a guideline that can be interpreted and applied differently by different people. It requires judgment and interpretation, and can even shift over time as your understanding of the problem changes.

In my Zellij geometric toy I needed to store things keyed by two-dimensional points. But the equality of points had to account for slight inaccuracies that could make “the same” point be not quite equal. I wrote the PointMap class to do that one thing. (Details of the algorithm are in my Finding fuzzy floats blog post.)

As I worked more on the project, I found that I wanted the internals of PointMap in more places. Rather than use that “fuzzy equality” only in PointMap, I needed to also use it to scooch the nearly equal points together so they were exactly equal. I refactored the code to create a Defuzzer class, which only handled the question of, “should this point be considered the same as some previous point we have seen?”

This new Defuzzer class was more useful to me. I could replace the old PointMap class with a Defuzzer and a standard dict, which was simpler.

So: was I wrong when I thought PointMap adhered to the Single Responsibility Principle? It looked good at the time, it was a nice tight class. But Defuzzer is smaller, does less, and is more useful. So it’s definitely a better poster child for SRP. Is one right and the other wrong? I don’t think we can make black-and-white statements like that. As I continue to expand the project, I might find reason to refactor Defuzzer even further. Where would that leave my stark assessment of its orthodoxy?

I’m all in favor of Single Responsibility. But I want people teaching it to help people more with the squishiness of it. Stop giving simplistic examples that clearly seem to violate the idea. Acknowledge the difficulty of knowing the right path.

Programming is hard. Principles and guidelines help, but when we present subjective judgment as binary choices that can be “violated,” we’re doing a disservice to ourselves and to new learners.

Comments

[gravatar]
Robert C. Martin also tries to define SRP as one function/class/component adheres to SRP when "it has only one reason to change". That statement helps when thinking about how much you should breakdown the thing you're dealing with. I totally agree it's not trivial, but the trick is to always pursue it. I believe SRP is the most important of the SOLID principles because it will help to define the others. If you sacrifice SRP, you'll probably be sacrificing the others too.
[gravatar]
@Gabriel, no fear, I am not sacrificing SRP, just trying to understand it. Frankly, "only one reason to change" helps me even less. Aren't there multiple ways you could need a class to change, even the most simple?
[gravatar]
Alistair Broomhead 9:31 PM on 28 Apr 2018
In Clean Code Robert C Martin explains this a bit more clearly, in terms of stakeholders, with the example of a payroll system.

The stakeholders in this would be a line manager who might make changes to an employee's pay grade, in the case of contractors the employee might need to log their hours, accounting might adjust where the pay bands sit, and HR might make alterations to holiday policies. Each class or function should only have the possibility of need for rework (changes in business logic) based on up to one of these stakeholders, otherwise it needs breaking up.

I'm still not sure this is perfect, but it's hopefully a little less vague than the other definitions?
[gravatar]
@Alistair: if the line manager can make changes to an employee's pay grade, or to their annual review rating, is that one thing or two things? It's one stakeholder still, right?
[gravatar]
Alistair Broomhead 10:36 PM on 28 Apr 2018
It's described as a single responsibility because it's a responsibility to only one external factor, or at least that would be my interpretation. Obviously SOLID is designed around OOP, and so you might have this be the responsibility of a single class, but have different methods or constructors for different use cases.

I have to admit nowadays I tend towards a more functional style where I can, which has me quite interested in python 3.7 dataclass objects. I try to keep SOLID principles in mind while knowing that they don't quite apply outside of classes.
[gravatar]
Interfaces are essentially borders, and where you draw a border is always political. Of course terrain affects borders, and likewise implementation requirements affect interfaces, but drawing a border at all is a political act. The political issue at hand in interfaces is what responsibilities (and obligations) one wants to assume and which ones one would rather punt on; also, what responsibilities one is willing to entrust to a dependency and which ones one wishes to have control over. (“One reason to change” is just an allusion to all of these things… without usefully naming any of them.)

The main implementation concern is “how much other code will need this functionality?”, but even then the shape of the interface is very much a political question.

This is somewhat of a non-answer, I am essentially saying that no easy definition of “does too much” exists. But I do think it helps make these calls once you stop searching for the shape of the interface (exclusively) within the piece of code you are writing… where it isn’t to be found. It’s in all the other code that surrounds that piece (and in the evolution of that other code over time).
[gravatar]
I got to read a body of code recently by someone who strongly adhered to the Single Responsibility Principle.

There were almost a hundred 40 line files, one 1000+ line file, and several multi-hundred line files.... and it was almost impossible to follow any coherent flow of logic as there would alway be some long routine at the top that was dipping in and out of these tiny little classes.

I believe that any practical real-world program has to have key services that violate the "Single Responsibility Principle." I vote we retire the SRP altogether, perhaps in favor of some idea like the "Clearest Coherent Unit Principle" - split your code into indivisible units, each of which is clear (easily understandable) and coherent (offers only a few services that work together).
[gravatar]
It's all about change. Refer https://vimeo.com/157708450
[gravatar]
@Tjorriemorrie: the Kevlin Henney talk you linked to was good. It talked about the subtleties, and he explicitly objected to "Single" in SRP, so we are thinking alike.
[gravatar]
Ultimately, it's an aesthetic judgement, and as you write a larger piece of code, you learn things about the task and your particular aesthetic judgement changes, such that different things look like a "single" responsibility.
[gravatar]
I feel better knowing that an experienced engineer had trouble with this concept! It gave me a ton of grief as well!

I finally came to terms with the SRP using the following thought process...

An entity's responsibility is only truly determined after the entity has been defined (usually as a result of negotiation between the developer and himself and others involved). The entity's responsibility may comprise completing multiple steps in a process (see "calculateAndDeliverPay") or maintaining the state of possibly multiple objects (in "has-a" relationships), but is singular in the sense that it is well-defined and comprehensible. It has only one reason to change: when the process it is responsible for must change.

You are violating the SRP when you mix two conceptually independent processes at the same level of abstraction under a single entity. If you add a new process to a function, you obfuscate its responsibility.

You might ask: If a single responsibility can comprise multiple steps in a process, why not just have one giant main function? My answer is: One giant main function is fine... IF your program does only one thing! If your program needs to be able to pay an employee and change the last name of an employee, then those processes need to be the responsibility of separate entities. These entities will then be subdivided into smaller entities. This where the judgment of a developer is crucial.

I will certainly agree that the amount of subjectivity involved here is not made clear in Clean Code. I had real trouble with this one... Hopefully, this explanation, while still not perfect, helps.

Add a comment:

Ignore this:
Leave this empty:
Name is required. Either email or web are required. Email won't be displayed and I won't spam you. Your web site won't be indexed by search engines.
Don't put anything here:
Leave this empty:
Comment text is Markdown.