Business logic and controller decoupling

2024-05-19 1092 words 6 minutes

Contents

Problem:

We have a problem regarding the separation between the business logic and the controller.
The business logic should be responsible for making the decisions and the controller should act on these decisions.
Lets highlight the problem by taking two examples an api and a normal crud.a

Api Example:

I have two doctypes Student and Course.
An api endpoint that calculate the fee that should be payed by the Student based on the number of Courses and type and the status of the student.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14


def calculate_fees(student_id):
    
  student = frappe.get_doc("Student", student_id) 
  if (student.status == 'active'): 
     courses =  frappe.get_all("Course", filters = {"student_id": student_id})
     amount = 0
     for course in courses :
        if course.type == 'virtual':
           amount += 50
        elif course.type == 'non-virtual':
           amount += 100
       
     student.amount = amount
     student.save()

In the above code the controller which is responsible for the flow of the request and the business logic responsible for the calculation of fee and communication with database are all in one function.
Drawbacks:
- As the code grows the function becomes more complex and unreadable.
- The function is hard to test, because it contains out of process dependency which is the database and thus it will need integration test with a real database connection to test the code.
- Weak protection against regression , because tests can’t cover all possible scenarios.
- Testing will be slow due to the presence of many integration tests instead of unit tests.
The code regarding the Course fee calculation should reside in the Course doctype and the controller shall ask for the fee of the Course without knowing about the calculation process by providing the data needed for the course.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10


    class Course(Document):
         
       def calculate_course_fee(self):

          if self.doc.type == 'virtual':
             amount = 50
          elif self.doc.type == 'non-virtual':
             amount = 100
 
          return amount

By doing this it becomes easier to test the calculation function and easier to spot errors like the one raised from the unhandled else.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14


  from course import Course

  def calculate_fees(student_id):
    
    student = frappe.get_doc("Student", student_id) 
    if (student.status == 'active'): 
        courses =  frappe.get_all("Course", filters = {"student_id": student_id})
        amount = 0
        for rec in courses:
            course = Course(rec)
            amount += course.calculate_course_fee()
       
     student.amount = amount
     student.save()

by doing the same for the student status, the controller becomes more focused on managing the flow and the connection between the collaborators, instead of making decisions which should be done by the domain logic.

Crud Example:

I have Four doctypes Exam, Student, Attendance, Class
I need to create an exam event for a student , taking in consideration the student should be enrolled in the class , the student status should be active , and the attendance percentage shouldn’t be less than 75% and the class should be completed.
The following code is for the validate function triggered by frappe before saving

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27


from frappe.utils import get_time, getdate, now_datetime

class Exam(Document):
   def validate(self):
       student_id = self.doc.student_id
       # get student record
       student = frappe.get_doc("Student", student_id) 
       # check student status
       if(student.status == 'active'):
       # get class record
         class = student = frappe.get_doc("Student", student.class)
         date = getdate(now_datetime())
         class_completed =  True if date > class.end_date else False
         
       # Calculate Attendance Percentage
       if class_completed:
         attendance_records = frappe.get_all("Attendance", filters= {"class": student.class, student: student_id})
         no_os_sessions = class.sessions
         attendance = 0
         for rec in attendance_records:
            if rec.present == 1:
               attendance +=1
           
         precent = ( attendance / no_os_sessions ) * 100

         if precent < 75:
            frappe.throw ("Student Should Be Deprived From Exam Access")

Drawbacks :

Same as above.
This approach—when a domain class retrieves and persists itself to the database—is called the Active Record pattern. It works fine in simple or short-lived projects but often fails to scale as the code base grows. The reason is precisely this lack of separation between these two responsibilities: business logic and communication with out-of-process dependencies.
The Student, class and Attendance doctype should be explicit and injected from outside , to make the exam doctype more testable.
Even if we refactor the doctype to contain only the business related as in the following:

1
2
3
4
5
6
7


   class Exam(Document):
     def validate(self):
        # check if exam is online
        exam = self.doc
        if exam.online :
           return True
           

This simple function is hard to test, because we need to have a real database connection to either mock an exam or even get a real one because the exam doctype inherits from Document class which requires a running frappe server , making it difficult to add unit test which we should use to verify the business logic.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10


   from exam import Exam
       
   class TestExam():
     # we need database connection and running server to work (integration test)
     def test_validate(self):
         exam = Exam("Final Exam") # which is a name of a record already exist in db.
         # or
         exam = Exam({ "capacity" : 20 , "online": True}) # which is a dict of the record we want to mock 
         assert exam.validate() == True
            

if we removed the business logic into a separate file , it will make the testing easier:

1
2
3
4
5
6
7


    class ExamLogic():
        def __init__(capacity, online):
            self.online = online
            self.capacity = capacity

        def validate_online(self):
            return self.online == True

1
2
3
4
5
6


  # compared to a unit test validating only the mock object only to Exam which is a use factory function or just a dict containing the exam info or even a parameterized info
  from exam_logic import ExamLogic
        
  def test_validate(self):
     exam = ExamLogic(capacity = 20, online = True)
     assert exam.validate() == True

This results into a simple test that is executed fast and focused on the business only with no need for the server to be running.

The solution of these problems won’t only benefit testing even if it does its still worth it , but it will result into more simple and clear code , where we refactor overcomplicated code by splitting it into algorithms and controllers. Ideally, you should have no code in the top-right quadrant.
By adhering to the single responsibility principle and open/closed, the code becomes more maintainable, extensible, and testable. It enables easier modification and extension of functionality without impacting other parts of the system, encourages separation of concerns, promotes code reusability, and facilitates easier testing.