Highly Opinionated Thoughts on Programming

by Elnur Abdurrakhimov


Keep Domain Logic Out of Controllers

Oct 31, 2015


The Single Responsibility of Controllers

To understand why domain logic doesn’t belong in controllers, it makes sense to understand what’s the main responsibility of controllers is.

The main and only responsibility of controllers is to map a UI to domain logic. That is, in the case of a Web application, controllers are responsible for parsing HTTP and HTML form requests, figuring out what a client intends to do, asking the domain layer to accomplish the task, getting the result of that task from the domain layer, composing a response, and sending it back to the client. That’s it.

If you follow the SOLID principle, you should know that S stands for SRP which stands for Single Responsibility Principle. The idea behind SRP is that a layer should have a single reason for a change. This principle is widely accepted when it comes to HTML/CSS. Remember the times when HTML was full of styling tags and attributes? It has changed. Now, most people define structure in HTML and styling in CSS. If you want to change the color of links, you do it in CSS. If you want to change the URL of a link, you do it in HTML.

But when it comes to controllers, most developers miss that separation. Controllers are full of code that doesn’t belong there. They make all kinds of decisions like parsing data from HTML forms, persisting separate domain objects, sending emails, making authorization decisions, and so on. Because of all that mess, controllers have a lot of reasons to change. And since controllers are so bloated, it’s hard to find the code you need to change when you need to change the behavior of your application.

Example of Doing It Wrong

Let’s look at a controller doing way too much:

public class SubscriptionController {
    private SubscriptionRepository subscriptionRepository;
    private ProrationCalculator prorationCalculator;
    private PaymentGateway paymentGateway;
    private Emailer emailer;
    // ...

    public HttpResponse changePlan(HttpRequest request) {
        Subscription subscription = getSubscriptionFromRequest(request);
        Plan newPlan = getPlanFromRequest(request);
        
        if (subscription.getPlan().getPrice() > newPlan.getPrice()) {
            subscription.setDowngradePlan(newPlan);
            sendDowngradeEmail(subscription.getCompany().getEmail(), newPlan);
        } else {
            subscription.setPlan(newPlan);
            int proratedAmount = prorationCalculator.calculate(subscription, newPlan);
            paymentGateway.charge(subscription.getCompany().getPaymentInfo(), proratedAmount);
            sendUpgradeEmail(subscription.getCompany().getEmail(), newPlan, proratedAmount);
        }

        subscription = subscriptionRepository.save(subscription);
        return createSubscriptionResponse(subscription);
    }
    
    private Subscription getSubscriptionFromRequest(HttpRequest request) {
        // ...
        return subscription;
    }

    private Plan getPlanFromRequest(HttpRequest request) {
        // ...
        return plan;
    }
    
    private HttpResponse createSubscriptionResponse(Subscription subscription) {
        // ...
        return response;
    }

    private void sendDowngradeEmail(String email, Plan plan) {
        Email email = new Email();
        // ...
        emailer.send(email);
    }

    private void sendUpgradeEmail(String email, Plan plan, Integer proratedAmount) {
        Email email = new Email();
        // ...
        emailer.send(email);
    }
}

This is a typical controller I see in all projects I either inherit or get involved with at a later stage of development. It’s a total mess, but doesn’t seem like that only because I’m showing just a single handler method here. Controllers I usually see are often over 500 LOC and some are even over 1K LOC. And most of those lines are not specific to dealing with UI like HTTP or HTML forms.

The controller in our example parses an HTTP request to figure out the subscription it’s going to deal with, the plan the subscription will be migrated to, and creates an HTTP response to send the resulting subscription back to the client. But it also does a lot of unrelated things.

First, it makes a domain logic decision about whether to upgrade or downgrade a subscription based on the price of the subscription’s current plan’s price and the new plan’s price.

Second, in the case of upgrading to a higher plan, this controller asks the proration calculator to calculate the prorated amount the client has to pay when upgrading, and then asks the payment gateway to charge that amount.

Third, it sends emails in both events — upgrading and downgrading.

Extracting Domain Logic

Now, here’s the favorite question I like to ask developers writing controllers like that: If you decide to add another type of UI, how much code would you have to duplicate to make it support all the existing features? Say, you have HTML UI and now you want to add a RESTful API supporting all the features the current UI does. If you have controllers like that, you’d have to duplicate a lot of code because, basically, controllers are the biggest chunk of your application’s code. Both HTML and API controllers will have differences in the code dealing with HTML forms or JSON/XML resources, but most of the code will be the same.

When you see so much duplication, it becomes obvious that all that common code has to be extracted somewhere to exist in a single place. But what is that place? The Service Layer is the answer.

Let’s create a subscription manager that will deal with domain logic regarding Subscription objects:

public class SubscriptionManager {
    private SubscriptionRepository subscriptionRepository;
    private ProrationCalculator prorationCalculator;
    private PaymentGateway paymentGateway;
    private Emailer emailer;

    public Subscription downgrade(Subscription subscription, Plan newPlan) {
        subscription.setDowngradePlan(newPlan);
        sendDowngradeEmail(subscription.getCompany().getEmail(), newPlan);
        return subscriptionRepository.save(subscription);
    }

    public Subscription upgrade(Subscription subscription, Plan newPlan) {
        subscription.setPlan(newPlan);
        int proratedAmount = prorationCalculator.calculate(subscription, newPlan);
        paymentGateway.charge(subscription.getCompany().getPaymentInfo(), proratedAmount);
        sendUpgradeEmail(subscription.getCompany().getEmail(), newPlan, proratedAmount);

        return subscriptionRepository.save(subscription);
    }

    private void sendDowngradeEmail(String email, Plan plan) {
        Email email = new Email();
        // ...
        emailer.send(email);
    }

    private void sendUpgradeEmail(String email, Plan plan, Integer proratedAmount) {
        Email email = new Email();
        // ...
        emailer.send(email);
    }

}

As you can see, it’s the same code except the code parsing HTTP because that’s exactly the responsibility of the controller. Let’s see what the controller looks like now:

public class SubscriptionController {
    private SubscriptionManager subscriptionManager;
    // ...

    public HttpResponse changePlan(HttpRequest request) {
        Subscription subscription = getSubscriptionFromRequest(request);
        Plan newPlan = getPlanFromRequest(request);

        if (subscription.getPlan().getPrice() > newPlan.getPrice()) {
            subscription = subscriptionManager.downgrade(subscription, newPlan);
        } else {
            subscription = subscriptionManager.upgrade(subscription, newPlan);
        }

        return createSubscriptionResponse(subscription);
    }

    private Subscription getSubscriptionFromRequest(HttpRequest request) {
        // ...
        return subscription;
    }

    private Plan getPlanFromRequest(HttpRequest request) {
        // ...
        return plan;
    }

    private HttpResponse createSubscriptionResponse(Subscription subscription) {
        // ...
        return response;
    }
}

Much better now, isn’t it? The controller became much shorter and simpler now. We can now call it a day and throw a party to celebrate our great refactoring skills. Right?

Not so fast, my friend.

Even though the mechanics of upgrading or downgrading a subscription are gone from the controller, notice that the controller is still the one making the decision about whether it’s an upgrade or downgrade. That’s domain logic and doesn’t belong in the controller.

Let’s fix it. First, we need to add a method to SubscriptionManager to migrate a subscription to another plan:

public class SubscriptionManager {
    private SubscriptionRepository subscriptionRepository;
    private ProrationCalculator prorationCalculator;
    private PaymentGateway paymentGateway;
    private Emailer emailer;

    public Subscription migrate(Subscription subscription, Plan newPlan) {
        if (subscription.getPlan().getPrice() > newPlan.getPrice()) {
            return downgrade(subscription, newPlan);
        } else {
            return upgrade(subscription, newPlan);
        }
    }

    private Subscription downgrade(Subscription subscription, Plan newPlan) {
        subscription.setDowngradePlan(newPlan);
        sendDowngradeEmail(subscription.getCompany().getEmail(), newPlan);
        return subscriptionRepository.save(subscription);
    }

    private Subscription upgrade(Subscription subscription, Plan newPlan) {
        subscription.setPlan(newPlan);
        int proratedAmount = prorationCalculator.calculate(subscription, newPlan);
        paymentGateway.charge(subscription.getCompany().getPaymentInfo(), proratedAmount);
        sendUpgradeEmail(subscription.getCompany().getEmail(), newPlan, proratedAmount);

        return subscriptionRepository.save(subscription);
    }

    private void sendDowngradeEmail(String email, Plan plan) {
        Email email = new Email();
        // ...
        emailer.send(email);
    }

    private void sendUpgradeEmail(String email, Plan plan, Integer proratedAmount) {
        Email email = new Email();
        // ...
        emailer.send(email);
    }
}

Notice that since we now have the migrate() method to make the decision on whether to upgrade or downgrade a subscription, we made upgrade() and downgrade() private. We do that to prevent the code using SubscriptionManager from calling those methods directly. It’s out of their concern. The only code to make that decision is the migrate() method of SubscriptionManager.

Let’s now look at the controller:

public class SubscriptionController {
    private SubscriptionManager subscriptionManager;
    // ...

    public HttpResponse changePlan(HttpRequest request) {
        Subscription subscription = getSubscriptionFromRequest(request);
        Plan newPlan = getPlanFromRequest(request);

        subscription = subscriptionManager.migrate(subscription, newPlan);
        return createSubscriptionResponse(subscription);
    }

    private Subscription getSubscriptionFromRequest(HttpRequest request) {
        // ...
        return subscription;
    }

    private Plan getPlanFromRequest(HttpRequest request) {
        // ...
        return plan;
    }

    private HttpResponse createSubscriptionResponse(Subscription subscription) {
        // ...
        return response;
    }
}

Much better now. All the controller does now is deal with HTTP request and response and calls a single method from the service layer. Everything else happens there.

If you now decide to add another type of UI for that feature, the only code you’d have to duplicate is the call to that migrate() method. That’s a completely different game compared to what we had initially.

Even though SubscriptionManager in the last example could be refactored even further, that’s a completely different story. Even if it stays as bloated as it is, it’s still much better than having all that code in a controller.

Another Benefit

Some people think that keeping domain logic out of controllers is beneficial only when an application has several types of UI like HTML and RESTful API. But it’s not true.

Here’s another very important benefit. When you write proper domain logic specifications or just tests, the layer verifications should run against is the service layer.

If you have domain logic in controllers, you don’t have a choice but to execute specifications against the UI. And it has at least two problems:

  1. Checks are slow because driving a UI is much slower than invoking the service layer directly, and
  2. You get false negatives when UI is broken even though the domain logic itself is correct.

As you can see, it makes sense to keep domain logic out of controllers even if you don’t plan to add another type of UI.



© Elnur Abdurrakhimov