3

Suppose I have the following class:

public class Course {
    // data
    public string Name { get; }
    public List<Student> Students {get;}
    //...

    // logic
    public int AverageGrade() {/* do something*/}
    public bool ReachedMaxStudents(){/* do something */}
}

I have some operations that interact with the database such as inserting a student to a class (should be done in the database as well.) What is the best approach to this?

First approach: developer has to know the course Id and pass it to repository.

public class CourseRepository : ICourseRepository{
    public void InsertStudent(int CourseId, StudentDto student){
        /* do something */
    }
}

Second approach: embed repository inside the domain object:

public class Course {
    private ICourseRepository _repo;

    // data
    public string Name { get; }
    public List<Student> Students {get;}
    //...

    // logic
    public int AverageGrade() {/* do something*/}
    public bool ReachedMaxStudents(){/* do something */}
    public void AddStudent(Student student){
        StudentDto s = /* map Student to Studentdto */
        _repo.Insert(s);
    }
}

What are the pros and cons to each approach and which one is preferred?

  • 6
    Any repository worth its salt would not require domain objects to possess knowledge of how to save themselves to the database. Ergo, I prefer the first approach. Let your repository worry about how to get the objects saved to the database; that's the whole point of having a repository layer. – Robert Harvey Oct 5 '17 at 20:18
  • @RobertHarvey Thank you. But here where the problem arise. Suppose that adding a student involves some logic (for example, the student should be an active student and should maintain a certain GPA and have the correct major, etc.) Should that be done in the domain object? how would the business object interact with the repository in this case? – Husain Oct 5 '17 at 20:24
  • 2
    You can have validation logic in your domain object; either add an IsValid() method or write a constructor that takes parameters and automatically constructs a valid domain object. – Robert Harvey Oct 5 '17 at 20:47
  • 1
  • 1
    See also: stackoverflow.com/questions/20035810/… – Doc Brown Oct 6 '17 at 6:30
6

No, domain objects should not have any idea of the existence of a repository, or even a repository interface. If you feel the need to do this, that means there is an underlying issue with your design (see below).

The root of your problem is that you have not modeled all of your entities properly.

A course does not have students it has registrations.

Not only is your missing entity causing your problems where you feel objects need to know how to persist themselves (they don't); it will cause you problems on edge situations... For example, what happens if your student registers for a course, then cancels their registration, but then changes their mind again and reregisters for a course? Your current model cannot handle this situation if it has to do any sort of historical analysis (perhaps there is a cancellation fee that needs to be levied).

You need at least six different classes here: Registration, Student, Course, RegistrationRepository, StudentRepository, and CourseRepository.

Make sure your repositories only try to save its direct entity... do not have the CourseRepository try to save all of a courses registrations.

Your code would like something like this...

//This would be a method in the UI, maybe called when a button is clicked... ect
void UI_RegisterStudentForCourse(Student studentToReg, int courseId)
{
    //Register student for course
    Course course = courseRepo.GetCourse(courseId);
    course.Registrations = regRepo.GetStudents(courseId);

    Registration reg = new Registration();
    reg.Course = course;
    reg.Student = studentToReg;
    reg.Date = DateTime.Now;

    if (course.CanRegister(reg)) // Check if the course is full, ect
       regRepo.Save(reg);
    else
       //Tell the user this registration could not be completed
}

The problem with the second approach you listed is, it makes your code very brittle. What if you need to run a simulation where things should not be saved? What if you want to run unit-tests against your registration logic? It would require extensive mocking. Additionally, when your code violates the Single Responsibility principal (mixing saving with domain logic in this case), it makes things difficult to maintain and changes to the code are much more risky.

  • This is all correct, nevertheless it IMHO fails to answer the main question: should domain objects hold references to repositories (repository interfaces)? For example, your code example above makes use of two repos. Does it belong into some method of a domain object? (My recommendation would be "no", but your answer leaves that out). – Doc Brown Oct 6 '17 at 6:38
  • @DocBrown Thanks for pointing that out, answer edited – TheCatWhisperer Oct 6 '17 at 14:06
  • Right, having the correct entity model is more important than the details of how code is structured. No amount of good code will fix an incorrect model. This is true in general of course. There might not be any silver bullets, and there certainly are no vampires, but there is a lot of bad thinking going on. – user251748 Oct 6 '17 at 14:30
  • @nocomprende I would be hesitant to call it "bad thinking" (but maybe that is just systematics)... I was better at modeling domains before I started coding professionally. It took me about six years to relearn to do it. We have a lot of bad concepts and frameworks that throw people off. Labeling a table "xref" or "join" makes people not think about the entity. Entity framework thoroughly confuses people as to what an entity is... ect... – TheCatWhisperer Oct 6 '17 at 14:35
  • OK, well, my phrase is a bit provoking, but you are correct that often the main enemy is terminology. I have seen many arguments over the definition of words, or where people did not even realize that the definitions differed. I think only one person actually understands Entity Framework (Julie / Julia Lerman) and everyone else is most likely lost. They have had to reinvent it at least once, and it continues to be a great solution which is way too powerful for most problems. Sometimes I think we would be better off just taking one expert as gospel and perfecting whatever technique they offer. – user251748 Oct 6 '17 at 14:52

Your Answer

By clicking "Post Your Answer", you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.