Search code examples
c#oopdependency-injectionsingle-responsibility-principle

Single Responsibility and dependencies


If an object has a Single Responsibility, can the following be acceptable:

public class Person
{
   public string Name;
   public DateTime DateOfBirth;

   private IStorageService _storageService;

   public Person(IStorageService storageService)
   {
      _storageService = storageService
   }

   public void Save()
   {
        _storageService.Persist(this);
   }
}

i.e. using a supplied collaborator (which also helps to stop the domain model being anemic).

Or should it be:

public class Person
{
   public string Name;
   public DateTime DateOfBirth;

   public Person()
   {
   }


}
public class StorageService
{
    public void Persist(Person p)
    {
    }
}

Solution

  • can the following be acceptable?

    class Person {
    Person(IStorageService) { } ...
    void Save() { } ...
    }

    This dependency doesn't make sense.

    While it doesn't strongly couple a Person to Storage, because it doesn't bind them to a specific storage implementation, I argue that any such dependency makes no sense.

    Methods as verbs

    Think of methods on a class as verbs that would be carried out by that type. You're telling an instance of that type to "do something", with respect to its local domain.

    What does it mean when I, as a person, Save?

    • I switched my insurance provider and reduced my costs by up to 15%?
    • I'm a redemptive deity?
    • I have downloaded my soul into an automaton?

    A storage service can and should Save. People cannot Save, and should not advertise that they can.

    Trying to shoe horn it in

    SaveTo might make more sense - i.e. public void SaveTo(IStorageService storage).

    But then you're saying a person is responsible for knowing how to save itself to storage. In my opinion, this is a violation of SRP. It is also shows a missing piece of Domain Analysis.

    The domain for a Person wouldn't contain anything about saving, storage, etc. It would contain interactions between people, and other things at that level of the domain. The domain of data persistence is a better place for a Save method.

    If Person is in the problem domain (at that level of abstraction), then Storage is in the solution domain.

    How you should separate your logic

    You have three pieces of logic here:

    1. Person - knows about "person things"
    2. Storage - knows about the particular type of storage, and how to access it
    3. Storage of Person - knows about how a person should be committed to storage

    Following my advice above, I'd leave Person to stand on its own. However, you can either separate the logic for Storage and Storage of Person, or you can combine them.

    The approach that ORMs take is to separate all three concepts. The "Mapping" in "Object Relational Mapping" is where "Storage of Person" is encapsulated.

    This approach allows your Storage logic to focus on the potentially complicated job of reading storage configuration, connecting to storage, ensuring storage is fast, choosing alternate storage methods, etc. It also removes any dependency on your main domain's model, so the storage code can be reused by any other domain model.