Business Rule Split among two classes

323 views Asked by At

I have a project allocation domain with the following business rules

  1. When a new employee is getting allocated to a project the total expenditure should not exceed the Budget Amount.
  2. For an employee the total allocation percentage should not exceed 100%

I have created entities as shown below created in C#.

QUESTION

The Allocate logic is split across two classes – Project and Employee..The List<Allocation> is passed as a parameter to the Allocate method rather than adding as property of the class... Is it correct approach or do I need to add List<Allocation> as property in these two classes?

Note:

Database

enter image description here

Entitles

enter image description here

Code

Project

 public class Project
    {
        public int ProjectID { get; set; }
        public int BudgetAmount { get; set; }
        public string ProjectName { get; set; }

        public void Allocate(Role newRole, int newPercentage, Employee newEmployee, List<Allocation> existingAllocationsInProject)
        {
            int currentTotalExpenditure = 0;
            if (existingAllocationsInProject != null)
            {
                foreach (Allocation alloc in existingAllocationsInProject)
                {
                    int allocationExpenditure = alloc.Role.BillRate * alloc.PercentageAllocation / 100;
                    currentTotalExpenditure = currentTotalExpenditure + allocationExpenditure;
                }
            }

            int newAllocationExpenditure = newRole.BillRate * newPercentage / 100;
            if (currentTotalExpenditure + newAllocationExpenditure <= BudgetAmount)
            {
                List<Allocation> existingAllocationsOfEmployee = GetAllocationsForEmployee(newEmployee.EmployeeID);
                bool isValidAllocation= newEmployee.Allocate(newRole, newPercentage, existingAllocationsOfEmployee);

                if (isValidAllocation)
                {
                    //Do allocation
                }
                else
                {
                    throw new Exception("Employee is not avaiable for allocation");
                }

            }
            else
            {
                throw new Exception("Budget Exceeded");
            }

        }
    }

Employee

public class Employee
{
    public int EmployeeID { get; set; }
    public string EmployeeName { get; set; }


    public bool Allocate(Role newRole, int newPercentage, List<Allocation> existingAllocationsOfEmployee)
    {
        int currentTotalAllocation = 0;
        if (existingAllocationsOfEmployee != null)
        {
            foreach (Allocation alloc in existingAllocationsOfEmployee)
            {
                currentTotalAllocation = currentTotalAllocation + alloc.PercentageAllocation;
            }
        }

        if (currentTotalAllocation + newPercentage <= 100)
        {
            return true;
        }

        return false;
    }

    }

References

Following is from Repository Pattern without an ORM

What behaviour is there that requires the customer to have a list of orders? When you give more thought to the behaviour of your domain (i.e. what data is required at what point) you can model your aggregates based around use cases and things become much clearer and much easier as you are only change tracking for a small set of objects in the aggregate boundary.

I suspect that Customer should be a separate aggregate without a list of orders, and Order should be an aggregate with a list of order lines. If you need to perform operations on each order for a customer then use orderRepository.GetOrdersForCustomer(customerID); make your changes then use orderRespository.Save(order);

3

There are 3 answers

3
Yugang Zhou On

The business rule is also relevant to the existing Allocations. What about making Allocation an Aggregate and wrap business rule in its Factory? like:

public Allocation Allocate(Project project, Role newRole, int newPercentage, Employee newEmployee)
{
     List<Allocation> existingAllocationsInProject = allocationRepository.findBy(project);
     //validate project rule
     List<Allocation> existingAllocationsInEmployee = allocationRepository.findBy(newEmployee);
     //validate employee rule
}

So in this case, we don't have to worry about how to find the existingAllocations. And the rules valiation could be further refactored using Specification patterns.

2
Gang Gao On

I have a few comments:

Separating the allocate logic is the right thing to do

Consider to move the allocate logic to service classes e.g. ProjectService and EmployeeService, so the domain models can be logic free

Consider to add a new AllocationService class to manipulate the allocations.

public void Allocate(Project project, Role role, Employee employee, int percentage)
{
      // Fetch allocation from allocation repository
      var allocations = _allocationRepository.GetAllocations(project.Id);

      // project allocation logic
      if (!_projectService.Allocate(Project, Role, int percentage))
      {
          // throw exception
      }

      // allocate to employee
      if(!_employeeService.Allocate(employee, role, percentage))
      {
          // throw exception
      }

      // create new allocation
      _allocationRepository.Add(new Allocation
            {
                ......
            });
}

The allocation repository and the services can be injected in via the constructor, e.g.

public interface IAllocationRepository
{
       IEnumerable<Allocation> GetAllocationsByProject(Project project);

       IEnumerable<Allocation> GetAllocationsByEmployee(Employee employee);

       void Add(Allocation);
}

The IAllocationRepository can be injected into EmployeeService and ProjectService as well so you don't need to pass the List of Allocation around.

0
jnovo On

The Allocate logic is split across two classes – Project and Employee..

I would not do this since it splits the allocation responsibility, thus breaking the single responsibility principle. If you find that it belongs neither to Project nor to Employee then a domain service may do the job. In general, operations involving several entities that do not form part of the same aggregate are candidates to be located in such a service.

The List<Allocation> is passed as a parameter to the Allocate method rather than adding as property of the class... Is it correct approach or do I need to add List<Allocation> as property in these two classes?

My answer would be neither those: add List<Allocation> only to your Project class.

I think that what you need to consider is what Allocation really stands for in your domain. Is it an entity that forms part of the Project aggregate? May it even be a value object instead of an entity?

Sometimes I find myself losing perspective of the domain when I have database relations around. In this case, I see that the Allocation table does not even have its own id; instead, it seems to represent just the relationship between Project, Employee and Role with several attributes. Although the domain model should not care about persistence, this may be giving some hints about what Allocation really represents.

From my point of view, an Allocation only makes sense in the context of a Project and thus it should be part of that aggregate. Arguably, its equality is not based on identity and thus, it may even be a value object. The responsibility of ensuring that the first restriction - not exceeding the budget upon allocation - is satisfied, belongs to the Project entity and it may be performed upon employee allocation.

The tricky constraint is the second one: an Employee not exceeding 100% allocation through several Projects. In this case, you may be interested in providing means to obtain those Projects for which a given Employee is allocated, maybe through your Project repository. You can also provide an operation to check to provide the total allocation for a given Employee, possibly through a domain service.

Note that you are actually doing all this logic in the Allocate method of your Project class: first you obtain all the Allocations through GetAllocationsForEmployee and then you pass the retrieved list to Employee.Allocate which could be actually be named CanBeAllocated. You may feel that it is responsibility of the Employee to ensure this business logic, but I think that it has little to do with neither its properties nor its behavior and thus, it should rather be part of the Project.Allocate method or a domain service if you keep feeling that there are mixed responsibilities.

As a final note, in case there is some confusion given previous comments, there is nothing wrong with putting logic inside your model classes, it is actually a fundamental part of the whole domain modelling! The AnemicDomainModel post by Martin Fowler provides some good insight into this.