If I want to save and retrieve an object, should I create another class to handle it, or would it better to do that in the class itself? Or maybe mixing both?
Which is recommended according to OOD paradigm?
For example
Class Student
{
public string Name {set; get;}
....
public bool Save()
{
SqlConnection con = ...
// Save the class in the db
}
public bool Retrieve()
{
// search the db for the student and fill the attributes
}
public List<Student> RetrieveAllStudents()
{
// this is such a method I have most problem with it
// that an object returns an array of objects of its own class!
}
}
Versus. (I know the following is recommended, however it seems to me a bit against the cohesion of Student class)
Class Student { /* */ }
Class DB {
public bool AddStudent(Student s)
{
}
public Student RetrieveStudent(Criteria)
{
}
public List<Student> RetrieveAllStudents()
{
}
}
How about mixing them?
Class Student
{
public string Name {set; get;}
....
public bool Save()
{
/// do some business logic!
db.AddStudent(this);
}
public bool Retrieve()
{
// build the criteria
db.RetrieveStudent(criteria);
// fill the attributes
}
}
3
Single Responsibility Principle, Separation of Concerns and Functional Cohesion. If you read up on these concepts, the answer you get is: Separate them.
A simple reason to separate the Student from the “DB” class (or StudentRepository, to follow more popular conventions) is to allow you to change your “business rules”, present in the Student class, without affecting code that is responsible for persistence, and vice-versa.
This kind of separation is very important, not only between business rules and persistence, but between the many concerns of your system, to allow you to make changes with minimal impact in unrelated modules (minimal because sometimes it is unavoidable). It helps to build more robust systems, that are easier to maintain and are more reliable when there are constant changes.
By having business rules and persistence mixed together, either a single class as in your first example, or with DB as a dependency of Student, you’re coupling two very different concerns. It may look like they belong together; they seem to be cohesive because they use the same data. But here’s the thing: cohesion cannot be measured solely by the data that is shared among procedures, you must also consider the level of abstraction at which they exist. In fact, the ideal type of cohesion is described as:
Functional cohesion is when parts of a module are grouped because they all contribute to a single well-defined task of the module.
And clearly, performing validations about a Student while also persisting it do not form “a single well-defined task”. So again, business rules and persistence mechanisms are two very different aspects of the system, which by many principles of good object oriented design, should be kept separate.
I recommend reading about Clean Architecture, watching this talks about Single Responsibility Principle (where a very similar example is used), and watching this talk about Clean Architecture as well. These concepts outline the reasons behind such separations.
12
Both approaches violate the Single Responsibility Principle. You first version gives the Student class to many responsibilities and ties it to a specific DB access technology. The second leads to a huge DB class which will be responsible not just for students, but for any other kind of data object in your program. EDIT: your third approach is the worst, since it creates a cyclic dependency between the DB class and the Student class.
So if you are not going to write a toy program, use none of them. Instead, use a different class like a StudentRepository for providing an API for loading and saving, assumed you are going to implement the CRUD code on your own. You might also consider to use an ORM framework, which can do the hard work for you (and the framework will typically enforce some decisions where the Load and Save operations have to be placed).
5
There are quite a few patterns that can be used for data persistence. There is the Unit of Work pattern, there is Repository pattern, there are some additional patterns that can be used like the Remote Facade, and so on and on.
Most of those have their fans and their critics. Often it comes to picking what seems to suit the application best and sticking with it (with all its upsides and downsides, i.e. not using both patterns at the same time… unless you’re really sure about that).
As a side note: in your example the RetrieveStudent, AddStudent should be static methods (because they are not instance-dependant).
Another way of having in-class save/load methods is:
class Animal{
public string Name {get; set;}
public void Save(){...}
public static Animal Load(string name){....}
public static string[] GetAllNames(){...} // if you just want to list them
public static Animal[] GetAllAnimals(){...} // if you actually need to retrieve them all
}
Personally I’d use such approach only in a fairly small applications, possibly tools for personal use or where I can reliably predict that I won’t have a more complicated use cases than just saving or loading objects.
Also personally, see the Unit of Work pattern. When you get to know it, it is really good in both small and large cases. And it is supported by a lot of frameworks/apis, to name EntityFramework or RavenDB for example.
3
If it’s a very simple app where the object is more or less tied to the datastore and visa-versa (i.e. can be thought of as a property of the datastore), then having a .save() method for that class might make sense.
But I think that would be pretty exceptional.
Rather, it’s usually better to let the class manage its data and its functionality (like a good OO citizen), and externalize the persistence mechanism to another class, or set of classes.
The other option is to use a persistence framework that defines persistence declaratively (like with annotations), but that’s still externalizing the persistence.
- Define a method on that class that serializes the object, and returns a sequence of bytes that you can put in a database or file or whatever
- Have a constructor for that object that takes such a sequence of bytes as input
About the RetrieveAllStudents() method, your feeling is right, it is indeed probably misplaced, because you might have multiple distinct lists of students. Why not simply keep the list(s) outside of the Student class ?
1