Search code examples
design-patternsdomain-driven-designsolid-principlesobject-oriented-analysissingle-responsibility-principle

What is the scope of the Single Responsibility Principle and how does it work with DRY?


I need help with clarifying my (mis)understanding of the Single Responsibility principle (SRP).

In many projects that I have worked on, my colleagues argue that the SRP means that a class should implement very limited functionality, e.g. calculating some totals over a collection of objects. This leads to classes that have one public method (one responsibility), e.g. CalculateTotals(...). I'm not a DDD expert, but from what I have seen this leads to Anemic Domain Model, DTOs and endless micro services.

Taking this approach and adding DRY to the mix leads to reuse of those classes in different parts of the application.

I tend to think of SRP on a higher level, the requirements level. For example, totals need to be calculated for reporting vs. totals need to be calculated for tax related calculations. Applying DRY in this situation can lead to unexpected bugs. When the totals calculation logic needs to change due to changes in reporting requirements, if we have applied DRY and reused the class, we will break our tax calculations.

Given that a reason for a change should come only as a result of a change in the requirements (I don't think that re-factoring applies here) should the DRY principle be scoped only within a single use case?

If the above statement is correct, does it mean that we don't have to break the tax calculation into a separate class? Well, we could do it to simplify the code, but would we not do for SRP reasons?

Am I wrong in my thinking?


Solution

  • What does the Single Responsibility Principle actually say?

    The Single Responsibility Principle doesn't say that a class should implement very limited functionality, it says that there should only be one reason for a class to change. So (at least as far as the SRP is concerned) it is fine for a class to have many methods if that class is only likely to change for one reason. Deciding whether the SRP applies (equivalently, deciding what constitutes a separate reason for a class to change) is, as with all such principles, a matter of taste and judgement.

    For example, in the original description of the SRP that I just linked to, I find the application of the SRP to a toy bowling game to be gratuitous -- the separated classes would all change if the rules of bowling changed. On the other hand separating geometric calculations from rendering a UI should make sense to anyone.

    I don't know your codebase, but a class for each method sounds wrong to me.

    What does the SRP have to do with DRY?

    The inverse of the SRP is that if two classes change for the same reason (as in that bowling game), maybe they should be one class. That in itself doesn't have to do with DRY at all (it's about diffusion of responsibility, not duplication of code), but: if someone forgot to DRY up when they wrote or extracted the second class, you will probably notice when you change both classes (or, worse, when you forget to change one and find out in production) and be reminded of why DRY is important.

    Should I deduplicate ("DRY up") my singly responsible classes?

    Yes. Copies of identical code lead to bugs. Don't be unreasonable; if extracting some piece of duplication makes your program harder to understand AND increases its number of lines of code, you're definitely overdoing it. But do extract significant duplication and you'll only have to test it once and change it once when it changes.

    However, always make it clear how an extracted method relates to requirements. If you need to sum some values in the same way for reporting and tax calculation, don't call the method totalForReporting or call it total and put it on your ReportingService and then call it from your TaxService -- someone who wants to change reporting and not tax calculation won't think to check whether that method is used in a context that its name would not lead you to expect. Instead, call the method something generic like transactionTotal, or just call it total and put it on your GeneralLedgerService, and it will be totally (ahem) clear what its role is and when it should change or not.

    Deduplicating with an eye on requirements will strengthen your domain model, because you'll always be careful to associate code with the appropriate domain concept, whether model or service or whatever.