Problem understanding the Single Responsibility Principle

Question

The SRP seens fairly reasonable when you first look at it. Every class should have one reason to change. Every class should take care of one thing. Right. But let's see this class:

Employee {
    CalculatePay();
    Save();
    DescribeEmployee();
    FindByID();
}

This class was used as an example by Robert C. Martin. He describes three responsabilities for this class; calculating pay that deals with accounting, save and find by id that deals with database & describe employee that deals with formatting.

My issue is that if you take these responsabilities and separate them into classes, then Employee will become a data structure. It won't do anything. If in the future you need it do something, you can certainly call that just another responsability and decouple it too. This means that essentially, this is procedural programming. You can say 'compose Employee of those classes and put wrapper methods in the Employee for them', but then you end up with chains of wrapping. Maybe it's good practice but it feels wrong.

Another issue is what you consider a reason to change. It can be anything. If you say 'this class changes when the program changes' then everything goes into a class. If you say 'this class changes when the last digit of the last serial code printed in the 23th of May, after a virgin has been sacrificed to the moon goddess, changes' then there's a trillion classes. Should I just guide myself by a class cohesion? Should I forget completely about this "one reason to change" buzzword?

Answer 1

After years of banging my head against the wall with the SOLID principles, I've been able to make sense of them by following these rules.

1. It's not a "reason to change" until it actually does change.
2. Two "reasons to change" are the same reason if they have a common cause.
3. A class that doesn't contain code with control structures is exempt from the SRP. ;

The first rule prevents you from over-generalizing and making things harder than they need to be. If you find yourself changing a class and later changing it again for a different reason, it's time to refactor. Until then, don't worry about it.

The second rule constrains the definition of "reason". For example, let's say that we want to show the number of years an employee has been with the company next to their name. We're not tracking years of service now, so we need to add it to Save(). We also need to update DescribeEmployee() to display the number. While two supposedly unrelated methods are affected, there's a common cause, so that only counts as one reason to change.

The third rule makes it okay to have a "Main" class that does nothing but configure the app, instantiating objects and passing them to other objects. This class will change all the time for all different reasons. Ideally, most of the changes to existing code will be confined to this class. That's a good thing because for all intents and purposes, a class without control structures is just a configuration file.

Answer 2

My suggestion is to replace the phrase "a single reason to change" with "a single goal/objective/requirement to fulfill".

In a goal model, each goal should be implemented by a separate module. Use either a Crews goal scenario model or KAOS goal responsibility model.

Answer 3

The key is in the name of the class (and perhaps a little in the context in which it is used).

If Person were used in a data oriented application it may indeed be little more than a data container without behavior. For behavior you would have classes like PersonFormatter, PersonPersister and SalaryCalculator.

Would your Person be an entity in a game, in which you would have a building with furniture objects and cleaners, guards, clecks and a delivery boy that shows up at the door at inconvenient times, you would have a different kind of person. It would be the base class for all people and feature behavior and capabilities like sit on chair, shout at you to leave, follow you around, capture you, ring the bell and so on.

The one-reason-to-change dogma I only heard about in one context: Robert Martin. I guess he coined it to make things easier. I would rather focus on dependencies directly and approach this from the other end. The point is to minimize dependencies so chances are you will not need to change your class if anything in your program needs to change. That is because you would only have to change a single class to solve your problem, which would likely not be yours. If any bug fix or feature request would result in the change of only one class, you would have done really well in the SRP department. This makes it easy to locate where you need to apply the modification and minimize the chances of breaking your application (of introducing new bugs), because the change would be relatively simple and (even more important) local. If you have a unit test for that class it would tell you things are still OK or not.

Answer 4

In your example you do not give the implementation of Employee, however we can guess that these methods do not actually do all the work themselves but call sub-component that have the responsibility of doing it. (eg employeeRepository save employee in db).

Here the responsability of Employee is to serve as an entry point or a facade for the operations done on Employee. Its role is to orchestrate this.

Answer 5

SRP is one of the trickier concepts. It is about structuring code that changes over time, in an evolving codebase. Code changes due to new requirements of the domain, but the structure of the code should also be adapted (especially over a couple of initial versions) as our understanding of the domain gets better, to better support the kinds of changes that are common or characteristic of the domain.

So, the "reason to change" reflects the underlying diving forces of change in the domain, whatever they might be. In a business domain, these may be the way they do business, the way they think about and organize concepts, the way the company is structured, etc. These will affect the change patterns and frequency of changes in the codebase. Some of it will be influenced to some extent by the tools, processes and paradigms you use as a developer, and the interaction between the domain experts and the development team. While you may be able to guess some of these driving forces given enough knowledge of the domain (and its dynamics), more likely you won't be able to get these right up-front, and you'll have to course-correct on the go.

The trick is to eventually structure the code (using SRP in combination with other principles) so that the kind of changes that are characteristic of the domain are made relatively easy, by decoupling code along those change axes. But, as with everything in design, there is some sort of tradeoff; the code will be coupled in other ways that make other kinds of changes harder - but that's OK (and that's why you need to course-correct, to hone in on the right¹ design).

Which means you'll have to develop the idea of what a "reason to change" is for that particular domain; you can't really determine different responsibilities by just looking at code without context, except in a very general way (that may have some utility when trying to explain the principle in an article, but doesn't necessarily map well to every situation).

If you have code that implements two or more responsibilities in this sense, then you will likely have both obvious and subtle dependencies among these, and changes in one will propagate across the system, and that's what you want to avoid, to the extent that it is possible to do so.

---

¹ "Right" here doesn't mean perfect or optimal - just one that works reasonably well.

Answer 6

The key to understand principles like SRP is to look at the purpose behind it. What is it supposed to achieve?

The purpose of the SRP is to make it less risky to make changes to code when requirements change.

"Reason to change" is about requirement changes. It is not about the phases of the moon or changes in data, it is only about the outside forces which can require you to make changes in the code.

In a real-world business application, requirements change all the time: A new style guide requires new colors on the login screen. A new pricing policy means all prices changes. Changes in tax regulations. A supplier upgrades the integration API. And so on. All these requirement changes will cause developers to change code. But ideally, each change should happen in a single isolated place and not affect any unrelated code. Changing the color of the login screen should not cause a bug in how taxes are calculated, because that code should not need to be changed or affected in any way.

Most people would know not put color rendering and tax calculations in the same class. But some examples might be more subtle. Lets say you use two external API's to fetch prices from external suppliers. These API's happen to be exactly alike, so you create a single client class and just pass a parameter which indicate which of the two endpoints to call. A good example of DRY, right? But the problem is that one API can very well change independently of the other, since these are independent suppliers. If one API change you have to change the shared client, and that would affect code for the API which is unchanged. You risk changing behavior of something which should not change. So this is a violation of the SRP.

You mention class cohesion - this is very good principle to follow. So is "Separation of concerns". When it comes down to it these principles say the same thing as SRP in different ways. I like the SRP because it puts the focus on the causes for change, which forces you to think outside of the code itself.

The thing that sometimes trip people up is that you might have code looking like this:

OnboardCustomer() {
  VerifyCreditRating();
  SaveAddress();
  DispatchWelcomeGift();
  SendWelcomeMail();
}

This code does a lot of different things obviously. But it is coordination code, which represent an on-boarding flow which is (hopefully) designed by a single authority. A change in how the welcome gift is dispatched will not change this particular code, only a decision to send the gift at a different point in the flow.

Of course the implementation of each of these methods should be in different classes. So you could call the workflow coordination class a "wrapper", but that would be misleading since it encodes some specific business logic (the order of events in the workflow) which is distinct from (and may change independently from) the concerns about how to check the credit rating and dispatch the gift and so on.