8

I have worked with examples like a Player object which knows how to change its own state.

Another example, is an Invoice object which knew how to calculate its invoice charges using an algorithm.

Does this violate the single responsibility principle ?

If yes, then is creating an InvoiceCalculator be the right approach to house the algorithm ?

And the PlayerStateChanger is that a valid thing.

Martin Schröder
  • 354
  • 1
  • 4
  • 19
GauravEdekar
  • 249
  • 1
  • 8
  • 5
    Possible duplicate of [How to determine if a class meets the single responsibility principle?](https://softwareengineering.stackexchange.com/questions/154723/how-to-determine-if-a-class-meets-the-single-responsibility-principle) – gnat Oct 23 '19 at 09:16
  • 3
    If class changing it's internal state would violate SRP. Then whole idea behind object-orientation is violation of SRP. – Euphoric Oct 23 '19 at 09:32
  • 1) ^What Euphoric said, and 2) there can be valid reasons to choose to structure your program or parts of it in a way that's different from the usual OOP approach; you can have data structures with no behavior, functions working with immutable data, etc. - and then apply SRP to individual functions or to composite units of code (a group of functions + the data they work on). It's about observing how different aspects of the code are coupled, and then deciding if decoupling them is a good idea based the responsibilities they serve. – Filip Milovanović Oct 23 '19 at 09:59
  • `Another example is an Invoice object which knew how to calculate its invoice charges using an algorithm.` I think this is a bad example because charges are usually calculated in different ways due to different reasons (it varies from country to country and from time to time). The way to calculate them changes oftener than the "data structure" that represents the concept Invoice. For this reason, we use to decouple algorithms from the model they operate with. – Laiv Oct 28 '19 at 15:41

3 Answers3

19

I have come to the conclusion that the "Single Responsibility Principle" is doing more harm than good due to its confusing name.

The principle does not say that a class should only be allowed to do only one specific thing. If that was the case it would be a really stupid principle!

An object knowing how to change its state is perfectly fine - indeed many will argue that only the object itself should be able to change its state. An invoice object being able to calculate invoice charges also sounds totally fine.

The SRP is about separating concerns which might change independently. An invoice object which contains logic for rendering itself as PDF and logic for calculating fees would be breaking the principle, since the look and layout of the invoice might change independently of the algorithm for calculating fees.

(Note that "change" here refers to changes in requirements which forces developers to modify code, not to runtime state changes which is a separate issue.)

The SRP may lead one down the road of defining numerous tiny classes just to be sure about the separation. But the principle should be balanced against its counterpart, that things which do change in unison should be coupled in the code. This is called "low coupling but high cohesion".

JacquesB
  • 57,310
  • 21
  • 127
  • 176
  • 3
    I would just like to add: when identifying responsibilities/concerns - it's not so much about what *might* change independently (although you can make reasonable guesses based on experience and common sense - like rendering vs calculating given in the example here), because in more subtle cases you are not going to get this right at the beginning; it's more about paying attention, and observing, during development, that two aspects of something *do* seem change independently, and adjusting the design on the fly, before it becomes too hard to do it. – Filip Milovanović Oct 23 '19 at 10:08
  • 1
    Thank you so much for this clear answer for this all too common confusion of “single responsibility” (responsibility= decision making source) with “do one thing” (which applies to individual functions and not to classes). – Christophe Oct 23 '19 at 17:59
  • 1
    [my thought as well](https://www.morganconrad.com/posts/srp) – user949300 Oct 24 '19 at 21:49
  • 2
    I really wanted to downvote this answer because of the first paragraph, but reading through the whose answer made me flip my choice to upvote this instead as it really does give a very good advice as to how to interpret SRP. The first paragraph though is somewhat contentious and argumentative... – Roland Tepp Oct 25 '19 at 07:11
3

I would advise against designing data objects (domain objects) that can change themselves.

I prefer to make the following distinction:

  1. Domain objects - Objects that carry data, e.g. Player, Invoice. Preferably immutable, but depends on the app (e.g. for a game you might need to have mutable objects for efficiency)
  2. Objects that contain business logic: InvoiceCalculator or a Bank object that deposits money into Accounts

Advantages:

  • You can pass domain objects to different threads without fear of race conditions or the need for thread synchronization, because they are immutable
  • You can serialize domain objects more easily
  • You can put them in sets and in maps as keys (if you implement equals / hashCode). Because they are immutable, you are safe to do.
  • By having similar logic in a business class, e.g. Bank class that withdraw / deposit money to an account:
    • You can easily find this functionality when looking for it
    • You can better control access to it from different threads

What I describe above has been characterized as the anemic domain model, by Martin Fowler. I prefer to call it separation of concerns.

Remember that programming is not a dogma, so don't get discouraged by those that might say "it's not true OOP". OOP is just one paradigm - there are also the procedural and functional ones.

When the article was written back in 2003, multi-core programs were not as common as they are today. Nowadays they are, so I like to keep my data objects simple and immutable and handle logic elsewhere.

Minas Mina
  • 152
  • 4
2

A software has many stakeholders and each of those stakeholders may request changes to it.

For example an invoicing application has several stakeholders:

  • The database administrator who is responsible about how the invoice data are stored
  • The Chief Operations Officer (COO) that gives the legal requirements about how each item is taxed, and how discounts are calculated
  • The accountant that decides how the data should be laid out in the invoice report printed

The aim of the Single Responsibility Principle is to help you from braking something of stakeholder B when making a change requested by stakeholder A. Single responsibility means that each class must be accountable to only one person (or better, role) in the company.

The last thing you want to do is have the COO yell at you for breaking how the tax is calculated, while migrating to a different db schema or while changing the layout of the report that the accountant asked you to.

Now, specifically for the invoice case in your example, the BCE model would recommend that you make a distinction between application agnostic logic, and application specific logic. An Invoice is a business (aka domain) object. This object might as well be used by other applications within the company that have to do with invoices. Methods such as addItem(), removeItem() belong to the invoice class (which is a business object) since they are use-case agnostic. Now, code with regards to some specific use-case, belongs to another class known as the controller (or interactor). I would say that calculating charges is part of the billing use-case and would probably put it separately. This is subjective, since one may also claim that that those rules are not specific to that one application or use-case.

Lefteris E
  • 129
  • 2