How to apply OOP to real world examples without putting all logic in Manager classes?

Question

I'm lately trying to implement a specific problem using an object-oriented approach. I get the main principles and its advantages, but I fail to apply it to a real world problem. Most examples one could find consist of Cats or Dogs being implementations of Animal. These however don't give me enough understanding how to model below problem regarding another frequent example: a school administration system.

Imagine a school having Students, Courses, Professors, and Notes. My implementation would be something like this:

class Person {
    string name;
    int age;

    Person(string name, int age) {
        this.name = name;
        this.age = age;
    }
}

class Student extends Person {
    double gpa;

    Student(string name, int age) {
        super(name, age);
    }
}

class Professor extends Person {
    string roomNumber;

    Professor(string name, int age, string roomNumber) {
        super(name, age);
        this.roomNumber = roomNumber;
    }
}

class Course {
    string name;
    Professor professor;
    Students[] student;

    Course(string name, Professor professor) {
        this.name = name;
        this.professor = professor;
        this.students = new Student[];
    }

    void enrolStudent(Student student) {
        students.add(student);
    }
}

class Note {
    Course course;
    Student student;
    double value;

    Note(Course course, Student student, double value) {
        this.course = course;
        this.student = student;
        this.value = value;
    }
}

Now the Student has a bunch of Notes and we want to calculate its GPA. This could be either straightforward averaging its Notes' values or more complex logic using weights and/or ignoring optional courses.

Now my question is: where do we put this logic? Ideally I would have a function double calculateGpa() on Student so you could call student.calculateGpa(), but having this logic on Student would break the SRP in my view. It also does not belong to any other class listed here. A class called GpaCalculator or NotesManager would be another guess but that seems to me too much like moving all the logic away from the domain and into classes that do not represent a real object but just actions (see also this answer).

If that would be the way to go here, why wouldn't I then just write a pure, static, stateless function in a class called NotesHelper? Creating a manager class to just have one function double calculate(), and using its instance instead of a static function feels to me like making it look like OOP while it isn't really. I feel like there should be a better approach, probably one I didn't think of, or maybe I am wrong here. Could you guys give me some pointers?

Answer 1

Indeed, many examples of inheritance are trivial and even misleading. Consider this - it's true that Professors and Students are both Persons, but does your program need to know that? As it stands, this is just unnecessary coupling between the classes.

Software is "soft" because it changes easily. Relatively, anyways. I assume you are not publishing a library, but an application with a user interface (graphical or otherwise). In this case, you can be flexible. No one else needs to know where the calculateGpa function is. If it ends up being too complex for Student, or changing too often, you can extract it then. Hindsight is 20/20, and in software we can adapt to that!

There is no "one true way" to model a domain - it always depends on which problem(s) you are trying to solve. But you probably don't know what problem you're solving, yet, so you should do the minimum viable thing that gets you closer to knowing the problem. The best design - at least for software you are maintaining - is usually one that is easy to change, not one that is maximally flexible. Being small helps a lot. So stay small until you have a reason not to.

Just put it in Student for now. Move it later, or don't.

Answer 2

My simple take on this is that GPA is a property of a student. Hence it should be calculated in the Student class. Student doesn't currently have any responsibilities — it's a pure data class — so I don't see how it breaks SRP.

Also:

• It seems calculateGpa should be simply gpa, replacing the field.
• There is a many-to-many connection between Student and Course, so it seems there is a missing Enrolment intermediary class holding the Note (which then no longer needs to be a class, because there's just the value field left).

Answer 3

In situations like this, I think it's less helpful to dwell on abstract models of the domain (domains can be modelled very differently depending on perspective) and more consider the work that needs to be completed and focus on encapsulation and - more specifically - the concept of tell, don't ask.

If I want to calculate a GPA, it's clearly not the Student's job to know how to do that. So I want a GPACalculator*, which I'm going to pass to the Student. The student can pass that through its Notes and each Note can report itself to the GPACalculator, which will calculate the GPA according to some algorithm that both Student and Notes are ignorant of.

Something along the lines of:

public class Student {
    private List<Note> notes;

    public void CalculateGpa(GPACalculator calculator) {
        foreach(var note in notes) {
            note.ReportValue(calculator);
        }
    }
}

public class Note {
    private double value;

    public void ReportValue(GPACalculator calculator) {
        calculator.AddNoteValue(value);
    }
}

public class GPACalculator {
    private List<double> values;

    public void AddNoteValue(double value) {
        values.Add(value);
    }
}

Then, you simply pass your calculator to your student and end up with a fully populated GPACalculator (either storing the note values as above and then calculating at the end, or calculating as you go, depending on what makes the most sense algorithmically).

Your GPACalculator is ignorant of your Student and its properties, your Student is ignorant of your GPA calculation.

When you want to retrieve your GPA, you can either expose a property from the GPACalculator or (better) have another method that allows you to pass another object for your GPACalculator to report its calculated GPA to.

"Tell, Don't Ask" is, for me, the essence of good OOP.

It's of course very important to remember that the storage model and the domain model are not necessarily the same as the concerns for the two things are very different.

(* In honesty, I would be tempted to ditch the "calculator" suffix and just call this class Gpa.)

Answer 4

I agree with Engineert on the have a property and delegate the calculation thing. Some remarks on your model:

You start with Person as a base class. You should ask yourself why. Yes, both students and teachers are people but is that relevant to the problem? OK, you have some common properties but will you ever in your application have to deal with people? Probably not.

You have a list of students in Course and a teacher. This seems wrong. During some time period/semester there will be a relationship between students and a particular course and one teacher will be teaching the course but students nor teachers are properties of the course. You are still missing a couple of classes here. One of those is a roster/schedule. This should reference students and courses and teachers, bring them together. For each period you will have new instances of roster, the course does not need to know about either (teachers, students or rosters).

This may be the most important question in OO development that you will be asking yourself a lot: "Does my class need to know about this?" Is it essential to its identity? Is it part of the things it needs to be a YourClassName ?

Answer 5

The problem I see is that you didn't design a model to reflect domain-specific behaviours. Instead, you did focus mainly on the data structures oriented to their persistence as an ER model.

While ER models' main goal is efficiency at doing CRUD, they are horrible at encapsulating invariants and domain-specific rules because most of the time we have to break encapsulation to make these data structures integrable with tools and libs. This sort of classes makes us break many of the OOP best practices what lead us to confusion regarding the logic and its location.

ER models also fail at describing responsibilities, abstractions and the relationship between these abstractions. Think about the relationship Student - GPA. What does the GPA say about the Person represented as Student? How GPA describe what Students are or how they behave? Is GPA to a Student what X, Y is to a Point? Does Student depend on GPA? Really? While the relationship suggested could make sense from the persistence standpoint, it makes very little from the domain.

So, what are we missing here? The GPA^{Grade Point Average} itself, but as a first-class citizen of the domain instead of a mere integer conveniently misplaced somewhere else.

The GPA is not a score. The score is its current state or its representation. The GPA is a formula. An aggregate that depends on the Student' Notes. A GPA probably doesn't make sense without Notes and Students, but these both makes sense without GPA.

Regarding changes, Student, Notes and the GPA are likely to change due to different reasons, due to different actors. What makes Student to change has nothing to do with the GPA. And vice-versa. Modifying the Student should not involve (in no way) the GPA. Modifying the GPA should not involve the Student or the Notes it belongs to.

If GPA and Student change and evolve independently, it's persistence can be independent too. While GPA is stored in memory as a function, on DB it can be stored as a score if we need to query by scores. If we don't, the GPA doesn't need to be persisted because as an aggregate, it can be calculated on-the-fly in run-time by fetching Notes.

Answer 6

I think you are confused about SRP. "GPA is a property of a student" as @l0b0 mentioned and Student class should keep this value. But it doesn't mean you should calculate GPA in this property. You can call another service method instead. When you do this, Student class can not be effected by changing calculation of GPA. It will be responsible of its properties.

public class Student
{
    private GPAHelper _gPAHelper;

    public Student(int studentId)
    {
        _gPAHelper = new GPAHelper();
        _studentId = studentId;
    }
    public Student(int studentId, GPAHelper gPAHelper)
    {
        _gPAHelper = gPAHelper;
        _studentId = studentId;
    }

    private int _studentId;
    public int StudentId
    {
        get
        {
            return _studentId;
        }
    }
    public double GPA
    {
        get
        {
            return _gPAHelper.calculateGPA(_studentId);
        }
    }
}

When you call a service class or a helper class, you give the responsible to this class. Calling helper or service class in Student class does not mean you give the responsibility of GPA calculation to Student. That's the point.

Someone doesn't like passing GPAHelper to Student class. I added another constructor.

Answer 7

A student is a person. It makes sense for Student to inherit from Person.

A professor is a person. It also makes sense for Professor to inherit from Person.

But a professor could also take classes, therefore a professor can also be a student. The inheritance paradigm of Student extends Person and Professor extends Person breaks down here.

If you separate the "Person" from the "Student" and "Professor" concepts you can have a professor who is also a student. This also helps solve your problem of where to put student logic.

A Student is the marriage of a Person and zero or more Courses. Composition is your friend here.

A Student is composed of a Person object and a collection of Course objects.

The GPA is a calculation of the cumulative grades received from all of the courses, including courses that are in progress. I would not say GPA is a property of student, but a function of the Student object. It should be a method.

The calculateGpa() method loops over all of the courses and tallies up scores for courses completed, as well as marks received in all of the student's courses that are currently under way, then returns the average of this calculation.

class Student
{
    private Person person;
    private Collection<Course> courses;

    // constructors

    public double calculateGradePointAverage() {
        if (courses.getCount() == 0)
            return 0;

        double sum = 0;

        for (Course course : courses) {
            // sum up grades and marks on assignments
        }

        return sum / courses.getCount();
    }
}

Answer 8

Disclaimer

I'm afraid there is no "right" or "wrong" here. There are just a bunch of opinions, and either way you will be able to find a sizeable group of people supporting the idea. As such, my answer here is not a "this is correct", but rather a "this works for me".

A direct answer to your question(s)

why wouldn't I then just write a pure, static, stateless function in a class called NotesHelper? Creating a manager class to just have one function double calculate(), and using its instance instead of a static function feels to me like making it look like OOP while it isn't really.

I agree and disagree at the same time.

It is correct that there is no difference concerning functionality. In this situation, a method or a static function will do the same thing, and apart from having to instantiate the class, there is no difference in usage either.

On the other hand, the OOP concept of encapsulation means caring about the interface and not about the implementation. All that we care about is that we call gpaCalculator.Calculate(...) to get the GPA. What GpaCalculator does internally - or if there even is an "internally" - doesn't matter to us. The class GpaCalculator does not have any fields and it could be a static function instead? We don't care.

To what end though?

The benefits of using an object here is in the encapsulation. This doesn't give you any advantage at the moment. It's more about the future.

Imagine the calculation gets more complicated in the future and GpaCalculator now does need a private field to do its job. You can add it with a constructor argument; now every place where it's constructed gives you an error and tells you "hey, I need that thing over here". This might only be one place.

However, every place where Calculate() is called will not need to change. And that makes sense, because those calling places haven't changed; they still just want to have the GPA calculated.

(Maybe GPA calculation doesn't change and it's a bad example, I don't really know the US school system. The general idea should be clear though.)

Additional info

Managers

Please put a lot of effort into avoiding "-manager" as a class name. It doesn't tell you anything about what it does, really. Just compare your own example; imagine how much another programmer knows if they see the class name GpaCalculator vs. NotesManager. Use a thesaurus, it's the most underrated programming tool.

Inheritance

Inheritance is way over-hyped in schools. I would advise to favour composition over inheritance. So, a Student and a Professor "have" a Person, instead of "being" one. It doesn't make a lot of sense in English, but it will save you some headaches. For example, a professor might take a course. You can then easily have a Professor and a Student both referring to the same Person object.

The "Uncle Bob" school of OOP

In the book "Clean Code", Robert Martin elaborates on a style of OOP that has worked well for me, and which I still use:

Objects hide their data behind abstractions and expose functions that operate on that data. Data structure[sic] expose their data and have no meaningful functions.

Well, "data structures" already has a different meaning in computer science, so I prefer to call them "data holders". Either a class holds data (in your example, Student, Professor, Course etc.), or it does something to data (e.g. GpaCalculator). When I say that this "works well for me", I mean it provides

• easier creation; it's pretty clear where new code goes, less issues with finding good names etc.
• easier change; it keeps things pretty easy to refactor (e.g. a change in GpaCalculator is pretty much unable to introduce errors in other objects that work with the same data)
• easier diagnosis; bugs are relatively easy to track down

Some people don't like this approach, but that seems to be restricted to people who haven't used it.