Domain Objects and How they Access the Database

Question

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?

Answer 1

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.