7

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?

8
  • 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. Commented Oct 5, 2017 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
    Commented Oct 5, 2017 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. Commented Oct 5, 2017 at 20:47
  • 1
    Releated: softwareengineering.stackexchange.com/questions/352156/…
    – Doc Brown
    Commented Oct 6, 2017 at 6:25
  • 1
    See also: stackoverflow.com/questions/20035810/…
    – Doc Brown
    Commented Oct 6, 2017 at 6:30

1 Answer 1

9

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.

9
  • 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
    Commented Oct 6, 2017 at 6:38
  • 1
    @DocBrown Thanks for pointing that out, answer edited Commented Oct 6, 2017 at 14:06
  • 1
    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
    Commented Oct 6, 2017 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... Commented Oct 6, 2017 at 14:35
  • 1
    Hi, does course.CanRegister(reg) need to access the database? If it needs to access the database, how can that be done? Thanks.
    – jflaga
    Commented Sep 26, 2019 at 12:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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