8

I'm trying hard to design following the SOLID principles. What I've found is that when you use the "Single Responsibility Principle" (the S of SOLID) you usually have to split classes between the data containers and the data processors. For example If I have a class person with 5 properties that is read from DB instead of putting everything inside a class I create a Person class with the properties and another PersonReader class that reads that information from the database and creates the Person.

If I do that I have to open the Person properties so PersonReader could access them but then I have less encapsulation than putting everything inside a black box and making the properties only readable.

Am I missing something or is this a drawback of this principle?

Thanks in advance

EDIT: I've changed the person writer to a person reader because there was no need to make property setters public at the beginning.

Ignacio Soler Garcia
  • 21,122
  • 31
  • 128
  • 207
  • The [Memento](https://sourcemaking.com/design_patterns/memento) design pattern is a classic solution to serialization/deserialization without violating encapsulation. That being said, modern tools just use reflection. – jaco0646 Mar 02 '20 at 19:51

4 Answers4

3

Most data access strategies (ORM techniques) involve compromises of the kind you're describing. The least intrusive ones use reflection (or introspection, etc.) to populate your entities when they need to (when no public setters exist, for example).

Having said that, if your entities are only data containers (as in the anemic domain model), the single responsibility principle shouldn't really concern you much, since your objects don't have complex logic that would be spoiled by (or interfere with) data access code.

Jeff Sternal
  • 47,787
  • 8
  • 93
  • 120
  • Thanks Jeff but I don't think it is an anemic domain model because I'm not splitting business logic from domain model, I'm taking out persistence from the domain model. Everything I've read takes me in that path, just read the example here http://p2p.wrox.com/content/articles/design-and-test-driven-development – Ignacio Soler Garcia Dec 10 '10 at 19:16
  • @SoMoS - sorry, I meant that I got the impression that you were already splitting business logic from the domain model (since you described the objects as mere containers). If that were the case, I wouldn't be too fussy about the SRP. Since it's not the case, it's a fair concern. – Jeff Sternal Dec 12 '10 at 15:14
3

Maybe I'm missing something, but why won't you write a constructor for Person class which accepts all it's properties? This way the PersonReader will be able to create a person without you opening the person's properties.

Another option is to mark the setter of the person's properties as internal (And make the assembly internals visible to the assembly of the PersonReader, if it's in a different assembly). This will allow you to access the person properties, but will prevent anyone to change them outside of the assembly.

ItayMaoz
  • 403
  • 2
  • 14
1

I definitely agree with you that sometimes you run into this problem. I've experienced it with serialization, which is similar to ORM. In my case I needed to write the state of an object to a (binary) file.

In some cases it can be appropriate to create an interface through which the internal state can be written and read. So your person class would get two metheds "save" and "load" (or "write" and "read" or whatever you think is appropriate). These methods are passed a "IPropertyWriter" and "IPropertyReader" respectively. (In my case I called them InStream and OutStream).

Person::save(IPropertyWriter writer) would then do 

writer.writeProperty("name", this.name);
writer.writeProperty("age", this.age);

You can argue that you are still violating the Single Responsibility Principle, but I'd argue that nobody else should know the internals of Person. (Especially in my case of Serialization, I had to store the internal state which is partly not accessible through getters). The main point is that Person is not coupled to any Database code. Your IPropertyWriter can be everything. Furthermore, the responsibility for "knowing its own properties" is not really a new property but is attached to every object anyways.

You might also ask the question how likely it is that Person will change. If it's very unlikely, I think friend classes like in C++ are a good way, but if it's likely to change, I'd rather have the serialization methods right at the place where the properties are declared so you don't forget to update the dependent code.

Philipp
  • 11,549
  • 8
  • 66
  • 126
0

You're thinking too granularly. A specific class should read the database- but you don't need a class whose sole job is to create a Person. That's just wasteful. A single database reader should be capable of creating any class that comes through the database- because that's it's job. To read from the database.

There's no encapsulation problems with opening Person's properties so that DatabaseReader can access them, as Person is redundant if no class is capable of creating it. It's an inherent part of any classes responsibility to be creatable and destructible.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • Opening a class allows others to modify its data when they aren't supossed to do that (even worse when they are public classes that can be used by 3rd party people that does not know that they have a public property that they shouldn't use it. – Ignacio Soler Garcia Dec 10 '10 at 21:33
  • Anyway I'm sorry but your answer has nothing to do with my question. I could agree that a Reader could read everything, the problem is still there, you have public setters when you would have only getters if the reader was inside the Person class. – Ignacio Soler Garcia Dec 10 '10 at 21:34
  • 1
    @SoMoS: Most languages provide more fine-grained control. In C++ you can "friend" classes and in C# you can make properties internal and have nested classes. I think that Java may have an equivalent too. public, protected, private aren't the only access controls. – Puppy Dec 10 '10 at 22:37