Reputation:
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)
{
}
}
Upvotes: 7
Views: 548
Reputation: 11
According to Single Responsibility Principle a class should have only one reason to change. Reason or source of changes are different business departments for whom we create and design different classes.
So to know if there is a SRP violation, designer needs to know different business departments he works for and designs classes such that no class has behaviour or state for more than one department. This may vary from business to business.
Though in first example given by you it is clear there can be two sources of change i.e. Customer manager(for managing customer name, birth date) and Database administrator/Schema Designer(saving customer to database). So it is clearly violating SRP.
In second example you are managing two different sources of change in two different classes.So, I would say second example is correct. Though I would change class name :).
Upvotes: 0
Reputation: 59111
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
?
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:
Person
- knows about "person things"Storage
- knows about the particular type of storage, and how to access itStorage of Person
- knows about how a person should be committed to storageFollowing 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.
Upvotes: 10
Reputation: 233135
If you carefully read the definition of the SRP, you'll notice that the definition of a responsibility is a reason to change.
The first version could have two reasons to change:
Thus, it doesn't adhere to the SRP, while the second version does.
Upvotes: 4
Reputation: 6090
If person is a domain object that you're modeling, I wouldn't advocate having it encapsulate a service. I mean, I don't know about you, but I'm a person, and I don't have a storage service ;)
I'd say the concern about an anemic domain model is a concern about how the domain objects relate to one another. It's anemic if they're simply property bags of literals. You probably don't want to remedy this by having them concern themselves with modeling things and also with figuring out when and how to persist themselves.
The anemic domain consideration would be remedied by having rich interactions. Perhaps you're modeling a school, and Person is an object that can be Student or Teacher and Class is a collection of students and a teacher. You'd then have some kind of concept of myTeacher.Assign(Homework hw, Class class) or something like that. This is how I would personally go about enriching my domain model - by modeling actual, conceptual interactions among your domain entities, rather than by 'modeling' how they interact with your data access plumbing code.
Just my two cents.
Upvotes: 0
Reputation: 26
I have found that the more complete and tested your Domain namespace is, the better the quality of your application. Persisting entities doesn't belong to the domain model, so I separate the two.
Upvotes: 0
Reputation: 1102
I would stick with the second version. If it has single responsibility, you could use the first version. But, in my mind, I like thinking of a persistence layer separate from model objects.
Also, you can serialize the 2nd version which can be helpful. You probably wouldn't be able to serialize the 1st version with the reference to the IStorageService.
Upvotes: 4