Saturday 28 April 2018 — This 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:
- Single responsibility principle
- Open/closed principle
- Liskov substitution principle
- Interface segregation principle
- Dependency inversion principle
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
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?
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.
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).
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).
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: