You can explain a code smell like fat controllers until you’re blue in the face, but some things just click better with pictures.

And to celebrate the newly available Thin Controller Recipes, I’d like to examine a particularly troubling issue for many .NET developers.

“I just don’t get the point of moving business logic from controllers into services…”

If it isn’t clear why you could benefit from biz-logic-free controllers, you may be missing some software design fundamentals that could make a boatload more sense after you read this article.

With that said, if the gurus are gonna preach the almighty state of thin controller-hood, we first have to understand why a fat controller is a code smell, what thin really means, and why it benefits us.

And, since thin is a relative adjective, we’ll use pictures (and some animated GIFs, woo!) to convey it.

Fat Controllers

Before I bust out my trusty Microsoft Pen, let’s look at some code so you can mentally map it to the pictures (and vice-versa).

Below is a theoretical controller for a bank loan application. This one processes requests for loans by calculating loan terms based on a the requested dollar amount and a person’s credit score.

public class LoanController : Controller
{
    private ILoanQuoteRepository _loanQuoteRepository;

    public LoanController(ILoanQuoteRepository repository)
    {
        _loanQuoteRepository = repository;
    }

    [HttpPost, Route("api/loans/quotes/")]
    public IActionResult CreateLoanQuote(LoanQuoteRequest request)
    {
        var quote = new LoanQuote(request.Amount, request.CreditScore);

        quote.CalculateLoanTerms();

        _loanQuoteRepository.SaveQuote(quote); 

        var response = new LoanQuoteResponse();
        response.Amount = quote.Amount;
        response.RequestStatus = quote.Decision;
        response.InterestRate = quote.InterestRate;

        return Ok(response);
    }
}

So, we start out with all of the logic implementing the quote creation business process inside the controller’s action method.

This is fat.

But it’s not fat in a vacuum, it’s fat relative to what it could be if SOLID principles were applied.

To be honest, the above controller is pretty tame, but the thing about business logic in the controller is, it encourages even more as time goes on. On a pinch, you or the next developer that needs to add some feature along the same code path will stick it riiight there. As will the next, and the next, and the next.

I don’t care for UML diagrams, and these soft pastels seem to cure my winter blues…

And just so we’re on the same page, purple represents the Controller class, green represents a controller action method, and the reddish-salmon color represents the logic.

Pay particular attention to the green actions as they are the best visual indicator of controller thickness (although the existence of other non-action methods in your controllers could also be a code-smell.)

So, why is this a code smell?

The definition of good code seems to change with the times. What was a good strategy 10 or 20 years ago is no longer as important today.

Today the focus is on flexibility.

Our software changes rapidly, so good code is code that is easy to change and code that is not so easy to break.

I can think of a several reasons code starts to smell around the fatness of the above controller:

1) There are many reasons it may need to change with actual code edits.

The controller is now the client of the repository, the request and response DTOs, and the LoanQuote domain entity, so if any of those change, it could percolate up and require the controller to change.

Change isn’t bad, of course, but it is risky. Can it be written in a way that is less risky? More testable, for example, or easier to understand?

2) You can’t test the controller in isolation.

To test the controller, you’d also have to test the LoanQuote’s CalculateLoanTerms() method.

3) The inclusion of domain logic invites the addition of more locally relevant logic by you and other developers.

I touched on this earlier. Big methods encourage laziness, and thus, even bigger methods.

Have you ever had to try and wrap your head around a behemoth method or a god object.

It ain’t fun.

4) Code inside a controller isn’t easily reusable.

I’ve changed my stance a bit on reusability, where I used to think it was a great thing to strive for in all cases, now – not so much. But, fat controllers do hint at the possibility that you’ve got logic for some process that you’ll have to write in multiple places.

Since controllers sit at the barrier between the user and the domain, and often applications have more than one way of executing the same task, you risk having duplication across each entry path.

What can we do about it?

This is where SOLID principles come in.

SOLID principles arose as a set of ideals that we can aim for to keep code adaptive and flexible.

The mantra “controllers should be thin” is actually a beckoning to abide to several SOLID principles in the context of a controller to fight against the issues I mentioned above.

It should be “Yo dawg, your controllers are doing TOO MUCH!”

In this case, we can benefit from the S , the O , and the D.

Thin Controllers via The Single Responsibility Principle

The Single Responsibility Principle (SRP) states that components should ideally only have one reason to change.

By shrinking the scope of an object’s duty, you compartmentalize behavior so that it is easier to reason about (making future changes easier), and you make it more stable because it shouldn’t need to be modified unless there is a feature or bug that involves that single responsibility.

When you have a controller with multiple responsibilities like in our example (mapping requests, responses, handling the quote entity and the repository), you have a large potential change surface.

I’ve used flashing red and green blocks to indicate areas of change. Green being code additions and red being existing code modifications or deletions.

How can we improve upon this design so that it isn’t so inviting to entropy?

Shrinking the controller’s change surface with application services

One immediate strategy is to simply move the logic that orchestrates the work between the DTOs and the LoanQuote into what is typically called an application service.

An application service can then be assigned the single responsibility of that task’s orchestration, leaving the controller to be a simple gateway between the HTTP request the service method.

You’ll notice a few things:

1) The controller (via the green action) is visibly thinner by a applying a better separation of concerns. That means the controller itself will be easier to reason about, and it won’t need to change as often.

2) We now have two objects instead of one. That does increase complexity, so there is a trade-off.

3) We’ve merely shoveled the girth of the controller somewhere else. Later you’ll see how application services can suffer the same problem of bloat as our controllers.

In code, we’ll create a service class that looks a lot like the original controller.

public class LoanService
{
    private ILoanQuoteRepository _loanQuoteRepository;

    public LoanService(ILoanQuoteRepository repository)
    {
        _loanQuoteRepository = repository;
    }

    public LoanQuoteResponse CreateQuote(decimal amount, int creditScore)
    {
        var quote = new LoanQuote(amount, creditScore);

        quote.CalculateLoanTerms();

        _loanQuoteRepository.SaveQuote(quote);

        var response = new LoanQuoteResponse();
        response.Amount = quote.Amount;
        response.RequestStatus = quote.Decision;
        response.InterestRate = quote.InterestRate;

        return response;
    }
}

And then shrink the controller down by merely delegating its old work to the new service.

public class LoanController : Controller
{
    private LoanService _loanService;

    public LoanController(LoanService loanService)
    {
        _loanService = loanService;
    }

    [HttpPost, Route("api/loans/quotes/")]
    public IActionResult CreateLoanQuote(LoanQuoteRequest request)
    {
        var loanQuoteResponse = _loanService.CreateQuote(request.Amount, request.CreditScore);

        return Ok(response);
    }
}

So, this is better, but can we add some flex to this design?

Programming to interfaces to support extension and further reduce the change surface

When creating application services, it is my preference to hide the implementation behind an interface to allow for easier testing, for extension via decorators, and to make my intention of loose coupling as clear as possible.

I started to animate the implementations overlaying the interface but it ended up looking like a whole lotta controller debauchery…

An interesting observation about this design is that, provided the interface’s method signature stays the same, your change surface is reduced to the injection point. In theory you’d only ever have to add new implementations or extensions rather than modify old ones.

I can’t say if it plays out that way very often in the real world, but it is a good example of the Open/Closed SOLID Principle.

Your controller can now stay unchanged, while your application service can be extended via decorators or simply swapped out entirely, all managed within your dependency injection container’s configuration site.

The interface:

public interface ILoanService
{
    LoanQuoteResponse CreateQuote(decimal amount, int creditScore);
}

The service simply implements the interface.

public class LoanService : ILoanService
{
   //...
}

And the controller specifies the interface for injection instead of the concrete implementation.

public class LoanController : Controller
{
    private ILoanService _loanService;

    public LoanController(ILoanService loanService)
    {
        _loanService = loanService;
    }

    //...
}

A drawback is that you do add complexity via more things to manage by creating an interface that only has one implementation.

A popular software engineering tenet is to program to abstractions, not implementations, but if we blindly put everything behind an interface, are we always better off? Some argue that if you find you do need the benefits of an interface, than you can always create one at that point in time.

I think that’s fair, but I sometimes will still work behind one to make my intent clear.

Okay, now that we have our task delegated behind an interface, we’ve got a much more flexible and modular design. The class responsibilities are better defined, and we’ve provided for extension points and we’ve decoupled the service implementation from the controller.

Is this all good, or can it sour?

Bloated application services

I mentioned above that application services can also bloat.

Instead of fat controllers, we get fat application layers.

I’d say the latter is better than the former simply because the responsibilities are clearer, but this outcome is most likely a result of the application service doing too many things.

The reality of controllers is usually far different than the simple single action controller that I showed at the start. We usually have at least a few actions, and each has potential to bloat.

Remember, the size of the green actions are the indication of fat. Disregard the purple controller class wrapping.

As you can see, once you’ve got a nice cozy home for that original orchestration logic, the tendency is to throw everything in there to satisfy all the work your controller needs to execute.

public class LoanController : Controller
{
    private ILoanService _loanService;

    public LoanController(ILoanService loanService)
    {
        _loanService = loanService;
    }

    [HttpGet, Route("api/loans/quotes/{id}")]
    public IActionResult GetQuote(int id)
    {
        var quote = _loanService.GetExistingQuote(id);

        return Ok(quote);
    }

    [HttpPost, Route("api/loans/quotes/")]
    public IActionResult CreateLoanQuote(LoanQuoteRequest request)
    {
        var loanQuoteResponse = _loanService.CreateQuote(request.Amount, request.CreditScore);

        return Ok(response);
    }

    [HttpPost, Route("api/loans/quotes/{id}")]
    public IActionResult DeleteQuote(int id)
    {
        _loanService.DeleteQuote(id);

        return NoContent();
    }
}

And the application service, handling it all:

public class LoanService : ILoanService
{
    private ILoanQuoteRepository _loanQuoteRepository;

    public LoanService(ILoanQuoteRepository repository)
    {
        _loanQuoteRepository = repository;
    }

    public LoanQuoteResponse GetExistingQuote(int quoteId)
    {
        var quote = _loanQuoteRepository.GetByID(quoteId);

        var response = new LoanQuoteResponse();
        response.Amount = quote.Amount;
        response.RequestStatus = quote.Decision;
        response.InterestRate = quote.InterestRate;

        return response;
    }

    public LoanQuoteResponse CreateQuote(decimal amount, int creditScore)
    {
        var quote = new LoanQuote(amount, creditScore);

        quote.CalculateLoanTerms();

        _loanQuoteRepository.SaveQuote(quote);

        var response = new LoanQuoteResponse();
        response.Amount = quote.Amount;
        response.RequestStatus = quote.Decision;
        response.InterestRate = quote.InterestRate;

        return response;
    }

    public void DeleteQuote(int quoteId)
    {
        _loanQuoteRepository.Delete(quoteId);
    }
}

When your application service has this many responsibilities, the benefits of putting it behind an interface are surely reduced.

Rarely would you decorate a single method on something that big, and swapping it out would be pretty unlikely too, as you’d need a reason to re-implement every method.

What can we do about this?

Breaking up fat application services via CQRS

A good solution to these fat service layers is to again apply the single responsibility principle and break them up into several smaller, single purpose classes.

A popular strategy that follows from that is command query responsibility segregation (CQRS).

Simplest way to explain it is that logic related to querying (database SELECT’s, really) is separated from logic that modifies (database UPDATE’s, INSERT’s, and DELETE’s)

Consider how most standard language APIs differentiate between the Reader’s and Writer’s when it comes to I/O. By keeping these tasks separate they are easier to reason about and modify and they won’t contain code that interferes with the other.

A very common trend I am seeing with CQRS is to use Jimmy Bogard’s Mediatr library and create small query and command classes that are propagated through an execution pipeline.

This article from Jimmy himself shows that technique.

I’ve yet to get my hands dirty with Mediatr, so I will instead discuss a simplified way of implementing CQRS.

Keeping CQRS simple just means keeping the query and command codepaths separate, and in relation to fixing our bloated application services, we can create separate classes for each query or command and inject them into the controller.

I also really liked Derek Comartin’s article on this.

Awww, they’re like cute little application service minis!

This aligns nicely with the standard HTTP verbs that controllers are responsible for.

Actions that handle commands typically work with HTTP POST, PATCH, DELETE, PUT (and maybe some obscure ones I can’t remember) and queries tend to be your GETs.

Here is a very basic code example of these kind of single purpose command and query classes.

First, one for the query:

public class QuoteQuery : IQuoteQuery
{
    private ILoanQuoteRepository _loanQuoteRepository;

    public QuoteQuery(ILoanQuoteRepository repository)
    {
        _loanQuoteRepository = repository;
    }

    public void Execute(int quoteId)
    {
        var quote = _loanQuoteRepository.GetByID(quoteId);

        var response = new LoanQuoteResponse();
        response.Amount = quote.Amount;
        response.RequestStatus = quote.Decision;
        response.InterestRate = quote.InterestRate;

        return response;
    }
}

One for the quote creation command:

public class CreateQuoteCommand : ICreateQuoteCommand
{
    private ILoanQuoteRepository _loanQuoteRepository;

    public CreateQuoteCommand(ILoanQuoteRepository repository)
    {
        _loanQuoteRepository = repository;
    }

    public void Execute(decimal amount, int creditScore)
    {
        var quote = new LoanQuote(amount, creditScore);

        quote.CalculateLoanTerms();

        _loanQuoteRepository.SaveQuote(quote);

        var response = new LoanQuoteResponse();
        response.Amount = quote.Amount;
        response.RequestStatus = quote.Decision;
        response.InterestRate = quote.InterestRate;

        return response;
    }
}

And one for the delete command:

public class DeleteQuoteCommand : IDeleteQuoteCommand
{
    private ILoanQuoteRepository _loanQuoteRepository;

    public DeleteQuoteCommand(ILoanQuoteRepository repository)
    {
        _loanQuoteRepository = repository;
    }

    public void Execute(int quoteId)
    {
        _loanQuoteRepository.Delete(quoteId);
    }
}

And to use them, we inject them into the controller and call the execute method upon the relevant object:

public class LoanController : Controller
{
    private IQuoteQuery _getQuote;
    private ICreateQuoteCommand _createQuote;
    private IDeleteQuoteCommand _deleteQuote;

    public LoanController(IQuoteQuery quoteQuery, ICreateQuoteCommand createQuote, IDeleteQuoteCommand deleteQuote)
    {
        _quoteQuery = quoteQuery;
        _createQuote = createQuote;
        _deleteQuote = deleteQuote;
    }

    [HttpGet, Route("api/loans/quotes/{id}")]
    public IActionResult GetQuote(int id)
    {
        var quote = _quoteQuery.Execute(id);

        return Ok(quote);
    }

    [HttpPost, Route("api/loans/quotes/")]
    public IActionResult CreateLoanQuote(LoanQuoteRequest request)
    {
        var loanQuoteResponse = _createQuote.Execute(request.Amount, request.CreditScore);

        return Ok(response);
    }

    [HttpPost, Route("api/loans/quotes/{id}")]
    public IActionResult DeleteQuote(int id)
    {
        _deleteQuote.Execute(id);

        return NoContent();
    }
}

Summary

Now we have a nice visible design evolution that shows the progressive thinning of a fat controller.

By applying the single responsibility principle to each area of bloat, we break up the complexity into smaller, easier to manage classes that can change in isolation.

We reduced the change surface to compartmentalized units that will ideally lead to a stabler codebase.

One where changes aren’t as risky and can be made with comparative ease.

If you’d like to go deep on applying the single responsibility to other controller tasks, you can hop on over to the original post, get on my personal list at the bottom and I’ll send you the Mapping DTOs recipe from the Thin Controller Recipe Set.