OPTEN, das einzige Umbraco-zertifizierte Unternehmen der Schweiz

Clean code - open/closed principle

The situation

The O in the SOLID acronym stands for the open/closed principle, this is the concept that a class should be open for extension but closed for modification. To explain what this means in practice I will present a real life example from a project which I worked on.

The scenario is a webshop which allows customers to create new mobile phone contracts. There is a class named CustomerContractService. The class contains an Add method which is responsible for saving a new customer contract to the database and adding it to the customer’s history, here is the method:

public CustomerContract Add(CustomerContract entity)
{
    CustomerContract newCustomerContract = base.Add(entity);

    if (newCustomerContract != null)
    {
        string historyMessage = "new contract created";
        AddHistoryItem(newCustomerContract, historyMessage);
    }

    return newCustomerContract;
}

New requirements

This method works great, but then the client comes to you and says that they have a new customer, the Acme corporation, they have their own Acme API, they want you to call their API and check that if an employee is ordering a new phone contract, that the employee has not been fired. They have had problems in the past of employees buying subsidised contracts after they have been fired. So the solution is easy, you just need to add a check in the Add method. The new code looks like this:

public CustomerContract Add(CustomerContract entity)
{
    if (workflow == Acme)
    {
        var customer = _customerService.GetById(entity.CustomerId);
        // add check here that the Acme user is allowed
        if (_acmeValidationService.CheckIfUserIsAuthorised(customer) == false)
        {
            throw new AcmeValidationException("acme user not authorised");
        }
    }
    CustomerContract newCustomerContract = base.Add(entity);

    if (newCustomerContract != null)
    { 
        string historyMessage = "new contract created";
        AddHistoryItem(newCustomerContract, historyMessage);
    }

    return newCustomerContract;
}

A bad smell

Things are working well, and then the client comes to you again with more good news, they have another new customer, the Tyrell Corporation. Great so now you have to add another check against another API, the class is growing and what if you need to keep adding new companies, something smells bad.

Open for extension

The reason for the code smell is that this class violates the open/closed principle. When the code needs modifying, the class is not closed, you have to go into the class and add a new company API call. The way to overcome this problem is to implement the open/closed principle, this means making the class open for extension, so that it does not need to be modified every time. To do this you add the virtual keyword to the method and remove the Acme check logic:

public class BaseCustomerContractService
{
    public virtual CustomerContract Add(CustomerContract entity)
    {
        CustomerContract newCustomerContract = base.Add(entity);

        if (newCustomerContract != null)
        { 
            string historyMessage = "new contract created";
            AddHistoryItem(newCustomerContract, historyMessage);
        }

        return newCustomerContract;
    }
}

Now you can easily make a new class named AcmeCustomerContractService which extends this class and here is where you add the check to the Acme API first, then call the base Add method if successful. This makes much more sense, the Acme logic is encapsulated in a custom class:

public class AcmeCustomerContractService : BaseCustomerContractService
{
    IAcmeValidationService _acmeValidationService;
    public AcmeCustomerContractService(IAcmeValidationService acmeValidationService)
    {
        _acmeValidationService = acmeValidationService;
    }

    public override CustomerContract Add(CustomerContract entity)
    {
        var customer = _customerService.GetById(entity.CustomerId);
        // add check here that the Acme user is allowed
        if (_acmeValidationService.CheckIfUserIsAuthorised(customer) == false)
        {
            throw new AcmeValidationException("acme user not authorised");
        }
    
       return base.Add(entity, saveToDb);
    }
}

Conclusion

It is so easy to do exactly the same thing for the Tyrell Corporation, create another new class the TyrellCustomerContractService.  The code is open for extension by being virtual, but closed for modification, we never need to actually change the code in the BaseCustomerContractService. This is the essence of the open/closed principle.


kommentieren


0 Kommentar(e):