A Weight Loss Guide for Fat Controllers

3 Steps to a Slim Controller

Ciprian

Ciprian

Software Engineer Team Lead at Softvision
Ciprian is a passionate software developer whose main concerns in a codebase are readability and maintainability - no matter the programming language in which they are written. As such, he has devoted his time to studying the universal language of design patterns that have proved themselves or continually emerge in the programming world, be them object-oriented or otherwise. With this knowledge of universal abstractions, his aim is to reconcile the existence and collaboration of functional, procedural and object-oriented programming inside the same codebases.
Ciprian

Latest posts by Ciprian

The world is faced with an obesity crisis and it seems that controllers, be them ASP.NET WebApi, MVC or other framework’s controllers, are no exception to this. In this article, I will articulate my strong opinion, weakly held on what a fit controller should look and behave like and why a fat controller can be (not necessarily) a bad thing.

Context

What is a fat controller?

Intuitively, we can all tell what a fat controller is – it’s a controller that simply does too much. More precisely, it does too much because it serves too many requests (has too many actions/endpoints) or it contains too much knowledge about the domain and/or the persistence layer.

But there is more to this than the fact that it is doing far too much – the application’s flexibility and ability to evolve is hampered in everything this controller touches. Unit testing becomes drudgery, as you have to fake HTTP requests and depending on how badly the logic is tangled, you may even have to fake database connections and responses.

When is it harmless?

It is very important to note that in some circumstances, it might actually be ok to have a couple of fat controllers. For example, small applications, toy projects for career self-development and basically anything else that is either going to be so simple that you won’t be able to build an incomprehensible controller, no matter how hard you try, or something that once built, nobody’s going to ever bother looking into your code.

In other words, having a fat controller is a trade-off: you’re giving away maintainability and readability for the sake of development speed and simplicity, which in some contexts might be desirable.

With this in mind, the rest of the article applies to scenarios where having a fat controllers is a big no-no: applications that you expect to undergo continuous maintenance and growth and applications where the standard of quality is high.

How does a controller get fat?

There are two main ways actually.

One is underestimating the scale and lifetime of the project. If you’re thinking that your application is going to be a small one and you can afford having no service layer and keeping all logic inside the controller, but time proves that the application is going to keep growing forever and ever, you might have well painted yourself into a corner.

Another way of doing it is one line of code at a time. When developing an application, if we are not disciplined enough, it is easy to fall prey to the temptation of stuffing business and/or persistence logic in our controller, since let’s face it, it’s the easiest thing to do.

I just need to check this before the service is called, I just need to store this once the service is called, I just need this extra method for this page on said controller, it’s a trivial operation – there’s no need to create a full-blown service for it and the list of reasons goes on. But the idea is that, unless you don’t know you shouldn’t be putting this kind of code inside of the controller due to various reasons (i.e: lack of experience, bad project management, feature creep), the controller primarily gains weight one snack at a time.

How to put a fat controller on a diet

Step 1: Restrict the number of actions it can handle by splitting it

The most common way of organizing actions I have seen is by the main type of entity they are working on, namely: CustomerController, OrderController, CartController, etc.

For small applications, this can be ok, since these controllers won’t have too many actions tied to them, simply because there’s nothing much to do even if they wanted to. But as soon as the application grows beyond a certain threshold, controllers start to become cluttered and have 1,000 lines of code, then 2,000, then 5,000 and then 10,000. Here we can see that psychological effect in action: bah, this controller already has 1,000 lines of code, what’s the problem if we add 500 more? Bah, this controller already has 7,000 lines of code, what’s the problem if we add 5,000 more? Using this line of thinking, it will never stop growing unless you act on it.

As is the case with other trouble in life, prevention is far easier than treatment, so when we see that the size of our controllers tends to get out of hand, the wisest course of action is to find a better way of organizing our actions and do it.

For example, the 10,000 LOC CustomerController could be broken down into CustomerDetailsController, RegistrationController, SubscriptionController, etc. Notice that even if the customer is at the heart of each business process in these controllers, by explicitly saying that this controller handles registration, they gain an identity of their own, allowing for much greater granularity. This is one of the fundamental refactoring principles in Domain Driven Design, the act of identifying business concepts that had been previously implicit and actually giving them a name of their own.

Again, if even this way of organizing actions does not seem enough, go even further. For example, if the CreateOrderController is too big, maybe it would be an idea to split it into CreateOrderType1Controller, CreateOrderType2Controller and so on.

As a side note, this approach can be successfully used on any class, be it a controller, service or model class, to reduce its scope to something more manageable by the human mind. Just substitute the “Controller” with “Service” or “Model” and you’re there.

Step 2: Cut back on cross-cutting concerns

I am sure that at least once in your career you have encountered a controller method that looks something like:

public IHttpActionResult DoSomething(SomeModel model)
{
    this.logger.Log($"Authenticating and authorizing for endpoint {nameof(this.DoSomething)}");
    if (this.authenticationService.IsAuthenticated(model) && 
        this.authenticationService.IsAuthorized(model))
    {
        this.logger.Log($"Entering endpoint {nameof(this.DoSomething)}");
        // Check something else
        if (model.IsValid)
        {
            // get something
            // do something else
            using (var context = new SomeEfContext())
            {
                // check something and do something else
            }
 
            this.logger.Log($"Leaving endpoint {nameof(this.DoSomething)} with 200");
            return this.Ok();
        }
    }
 
    this.logger.Log($"Leaving endpoint {nameof(this.DoSomething)} with 300");
    return this.BadRequest();
}

Now, for the sake of this article’s readability, I’ve condensed all the juicy logic either under a service or under a set of comments, but I think you can easily imagine or recollect how such a method could end up having hundreds of lines of code.

The first and foremost thing we could pull out to lighten the load on the controller are cross-cutting concerns. For example, in the case of WebApi, we can safely pull the logging, authentication, and authorization to some IFilter implementations.

[LogEntryAndExit]
[Authenticate]
[Authorize]

public IHttpActionResult DoSomethingWithoutCrossCutting(SomeModel model)
{
    // Check something else
    if (model.HasSomething && model.CanDoSomething)
    {
        // get something
        // do something else
        using (var context = new SomeEfContext())
        {
            // check something and do something else
        }
 
        return this.Ok();
    }
 
    return this.BadRequest();
}

This way, not only have we removed all that clutter from our code, but we have also encapsulated it in a set of classes that we can now reuse in other controller methods.

Step 3: Cut back on domain and persistence logic

In a large enterprise application, no matter what sort of architecture you are using, it is a very good idea to have your business logic separated into its own project, separate from any presentation and persistence framework. This allows you to more easily and more effectively unit test the code that actually does the heavy lifting. Furthermore, it prevents you from being vendor-locked-in, being forced to use a certain framework just because your codebase is so dependant on it, there’s no feasible way to give it up.

In the code above, we have neither of these benefits. If we wanted to use something else than WebApi, we couldn’t do it straight since part of the domain logic leaked into the controller. We’d have to either reproduce that logic inside that new framework adaptation or refactor and put the domain logic inside a service. The same applies for persistence – switching from Entity Framework to something else will be a long and strenuous process.

In order to regain the benefits of said architecture, all the domain and persistence logic must be abstracted behind some interface that the controller will use. Now, since the domain will be doing all the interesting decisions, with the controller only supplying the parameters, all that is left for it to do is transform the responses it gets from the domain (be it through exceptions or other mechanisms) into WebApi specific responses.

[LogEntryAndExit]
[Authenticate]
[Authorize]
public IHttpActionResult DoSomethingWithZeroBusinessLogic(SomeModel webApiModel)
{
    SomeDomainModel someDomainModel;
    try
    {
        someDomainModel = new SomeDomainModel(webApiModel.PropA, webApiModel.PropB);
    }
    catch (DomainValidationException e)
    {
        return this.BadRequest(e.Message);
    }
 
    try 
    {
        this.someService.DoStuff(someDomainModel);
    }
    catch (BusinessException1)
    {
        return this.Conflict();
    }
    catch (BusinessException2)
    {
        return this.NotFound();
    }
 
    return this.Ok();
}

As you can see, this method now has a cyclomatic complexity of 1 – it executes no logic in by itself and all it does is translate the responses (both return value and exceptions) it expects from the domain into corresponding http responses. Also, note it is only catching exceptions that are exceptions to the happy flow, anything else outside the contract of that service will bubble up past the catch clauses. This is a quite standard usage of exceptions for business flow branching (an approach I do not condone, but which I included for the sake of making the article as accessible as possible).

Another example, if you prefer a more functional-oriented style, using a discriminated union:

[LogEntryAndExit]
[Authenticate]
[Authorize]
public IHttpActionResult DoSomethingFunctionalStyle(SomeModel webApiModel)
{
    var domainRequest = new SomeDomainModel(webApiModel.PropA, webApiModel.PropB);
    var domainResponse = this.someService.DoStuffFunctionalStyle(domainRequest);
    return domainResponse.Match<IHttpActionResult>(
        success => this.Ok(),
        notFound => this.NotFound(),
        alreadyExists => this.Conflict(),
        otherBusinessFailure => this.BadRequest());
}

What a slim controller looks like

We’ve now seen how a fat controller looks and what we can do to make it slimmer. So, we know where we start, what paths we can choose, but we don’t know the destination yet.

In my opinion, and from the perspective of maintainable and readable code, a controller, no matter the framework, should be nothing more than a mere mapper – it maps application requests to domain requests and domain responses to application responses (as in the example above). Anything more than that implies that the controller knows too much about the domain and is insidiously gluing itself to the core business logic (as in the initial example).

Conclusion

If you are working on an enterprise application that is expected to be under development and maintenance for a while, I strongly recommend that you try to avoid falling into the trap of laziness, let business/persistence code live inside your controller and, in the end, let it decide what you can and can’t do to your code.

Let the domain do all the decision-making and write slim controllers that act as simple mappers (adapters) between your presentation framework of choice (WPF, MVC, WebApi, Console, etc.) and your domain and between your persistence technology and domain. Although at first this might seem like “just another layer of abstraction”, the benefits of being able to thoroughly unit test your domain and have complete freedom of choice when it comes to the presentation and persistence layer is going to be an immense gain in the long run.

Share This Article


Ciprian

Ciprian

Software Engineer Team Lead at Softvision
Ciprian is a passionate software developer whose main concerns in a codebase are readability and maintainability - no matter the programming language in which they are written. As such, he has devoted his time to studying the universal language of design patterns that have proved themselves or continually emerge in the programming world, be them object-oriented or otherwise. With this knowledge of universal abstractions, his aim is to reconcile the existence and collaboration of functional, procedural and object-oriented programming inside the same codebases.
Ciprian

Latest posts by Ciprian

No Comments

Post A Comment