7

I am trying to apply "SOLID" whenever I can and try to use common sense and avoid a pattern when I see that a pattern is creating more problems than it's trying to solve. I don't want to apply a pattern and make life difficult for somebody else using my code just for the sake "I write patterns" if you see what I mean.

Now I am struggling with one of the principles that I thought was the easiest to grasp: "SRP".

How do you practically apply this principle to repositories?

Let's suppose I have a

IEmployeeRepository
IUserRepository
IProductRepository

and commonly they will have methods like these:

public interface IUserRepository
{
    User GetUser(int id);
    IEnumerable<User> GetAllUser();
    void DeleteUser(int id);
}

same for employees and products.

Are we saying that each of these method should be a class on it's own? even though at times we are talking a single line of code?

gnat
  • 21,213
  • 29
  • 113
  • 291
developer9969
  • 181

3 Answers3

13

Whenever you have a class with more than 1 method, you can question if the SRP is fulfilled, since each of the methods will (typically) solve a different task or problem and so has a "different responsibility". But that's actually not the way I understand the SRP - SRP means IMHO "single responsibility at the correct level of abstraction". And the level of abstraction for a repository may be

"repository RX is responsible for providing abstract CRUD operations for class X"

In fact, when you start implementing such a repository, it may turn out that some of the member functions of the repo may become so complex that you need additional helper classes to implement them. If that's the case, the repo may turn into a facade, but the responsibilty stays the same - at that level of abstraction.

In fact, there is no "absolute rule" how to choose the "correct" levels of abstraction. There are guidelines like

  • the "one reason to change" mantra, or

  • simple indicators like the code size, number of parameters of certain methods or

  • the problem of coming up with a concise name, or

  • the possibility of distributing the maintenance, evolvement and requirements management of pieces of software to different people or organizational units (citing Bob Martin: "This principle is about people")

But at the end of the day, this is up to your experience and to some degree of your taste.

Doc Brown
  • 206,877
  • 1
    That SRP exists implies the existence of some general rule or guideline. Unless, of couse, it is purely a guideline with no representative examples. That would make it useful for pondering or discussion, but not much else. – Robert Harvey Nov 02 '14 at 18:40
  • @RobertHarvey: I rediscovered your comment above today, and edited my answer accordingly - better now? – Doc Brown Jan 08 '21 at 10:17
  • Yes, better. ... – Robert Harvey Jan 08 '21 at 17:28
  • 1
    The official guideline for who chooses the level of abstraction is that it is the "owner" of that feature. “Responsibilities are People” in the sense that they tend to map to people with individual roles. People using or managing the software. In this case, the person (or team) who made the requirement that stuff get persisted in a Database / Local XML File / Cloud etc... – user949300 Jan 08 '21 at 19:04
  • 1
    @user949300: that's a good remark, I tried to integrate this into my answer right now. – Doc Brown Apr 05 '21 at 08:50
9

I think you should think who has an interest to change that respective class. For example if you have an User class with an CalculatePay method and a save method. The accountant would want to change the CalculatePay and the DB administrator the save method. These are two different responsibilities.

The SRP means grouping together things that change for the same reason. So the repository shouldn't violate the SRP.

thesolidphp
  • 91
  • 1
    +1. Imo, the best formulation of SRP is "a class should have only one reason to change". In the case of the code in the question, the only reason for changing would be if details of the storage mechanism changed. – Jules Nov 29 '14 at 14:10
0

It's been forgotten in the mists of time, but the original basis for Uncle Bob's SRP was based on the 1994 work of David L Parnas. From page 24:

We propose instead that one begins with a list of difficult design decisions or design decisions which are likely to change. Each module is then designed to hide such a decision from the others.

IMO, this wording is still much more clear and understandable than most of the "SRP principles" since. Consider IUserRepository

  1. Was anything in GetUser, GetAllUser, or DeleteUser a "difficult design decision"? Probably not. In that case, no need for an individual module.
  2. Is anything there "likely to change"? Possibly the storage mechanism, or the query mechanism (id changes from int to a long or a String) so perhaps, as a group, this deserves a module.
user949300
  • 8,823