Controllers, The Other Billion Dollar Mistake

First, if you are a seasoned engineer, you might have spotted the obvious analogy with Tony Hoare's Null References : The Billion Dollar Mistake.

In this article, we will see that like "Null References", "Controllers" are probably as bad because they are boilerplate, they spread logic in multiple places and last but not least, they make testing very cumbersome.

In the MVC pattern, the controller's responsibilities are usually limited to :

• Reading request data (params, query, headers, body)
• Calling one or more services
• Translating service results into HTTP responses
• Mapping domain errors to HTTP status codes

A typical example

Let's imagine a business app where we want to manage users : list, create and delete.

Although it is not in the "MVC" pattern per sé, the service layer is often used to define business logic, especially when it touches multiple models. Let's define a basic service implementing our business features :

[🔖 services/UserService.ts]

export class UserService {
    constructor(userModel) {
        this.userModel = userModel;
    }

    async list() {
        return this.userModel.findAll();
    }

    async create(data) {
        return this.userModel.create(data);
    }

    async delete(id) {
        return this.userModel.delete(id);
    }
}

To be able to use these features, we need to expose these methods in the controller (using hono) :

[🔖 controllers/UserController.ts]

import { Context } from 'hono';

export class UserController {
    constructor(userService) {
        this.userService = userService;
    }

    async list(c) {
        const users = await this.userService.list();
        return c.json({
            users,
        });
    }

    async create(c) {
        const data = await c.req.json();
        const user = await this.userService.create(data);
        return c.json({
            user,
        });
    }

    async delete(c) {
        const id = c.req.param('id');
        const deleted = await this.userService.delete(id);
        return c.json({
            deleted,
        });
    }
}

We can already see a pattern here. We have a 1:1 relationship between the service layer and the controller layer.

And we are not done. Just like the service methods need to be exposed, so do the controller methods. For them, we need a router (still using hono) :

[🔖 router.ts]

const app = new Hono();

const controller = new UserController(new UserModel());

app.get("/users", (c) => controller.getUsers(c));
app.post("/users", (c) => controller.createUser(c));
app.delete("/users/:id", (c) => controller.deleteUser(c));

Once again, we repeat our business features in another layer : the routing, which can be considered as part of the controller layer.

To summarize, at this point, for a given business feature, we have to define :

• a method in the model
• a method in the service
• a method in the controller
• a declaration in the router

What do the "DRY" gurus think about that ?

Another annoying thing is that we put all of them in the same "bag" while they have no relationship between them, besides acting on the same model. It is no wonder that in some codebases, some services and controllers contain thousands of lines and are totally unmaintainable.

Adding authentication

Right now, our business features are "open bar" : anyone can list, create and delete. We want to limit each one like so :

• list : everyone authenticated
• create : anonymous
• delete : admin only

Where should we define this ? In the router as a middleware ?

[🔖 router.ts]

-app.get("/users", (c) => controller.getUsers(c));
+app.get("/users", requireAuthenticated(c), (c) => controller.getUsers(c));
-app.post("/users", (c) => controller.createUser(c));
+app.post("/users", requireAnonymous(c), (c) => controller.createUser(c));
-app.delete("/users/:id", (c) => controller.deleteUser(c));
+app.delete("/users/:id", requireAdmin(c), (c) => controller.deleteUser(c));

In the controller ?

[🔖 controllers/UserController.ts]

export class UserController {
    constructor(userService) {
        this.userService = userService;
    }

    async list(c) {
+       if (!c.req.auth) {
+           return c.json({ error: 'Unauthorized' }, 401);
+       }
        const users = this.userService.list();
        return c.json({
            users,
        });
    }

    async create(c) {
+       if (c.req.auth) {
+           return c.json({ error: 'Forbidden' }, 403);
+       }
        const data = await c.req.json();
        const user = this.userService.create(data);
        return c.json({
            user,
        });
    }

    async delete(c) {
+       if (!c.req.auth || c.req.auth.role !== "admin") {
+           return c.json({ error: 'Forbidden' }, 403);
+       }
        const id = c.req.param('id');
        const deleted = this.userService.delete(id);
        return c.json({
            deleted,
        });
    }
}

In the service ?

[🔖 services/UserService.ts]

export class UserService {
    constructor(userModel) {
        this.userModel = userModel;
    }

    async list(auth) {
+       if (!auth) {
+           throw new UnauthorizedError();
+       }
        return this.userModel.findAll();
    }

    async create(auth, data) {
+       if (auth) {
+           throw new ForbiddenError();
+       }
        return this.userModel.create(data);
    }

    async delete(auth, id) {
+       if (auth?.role !== "admin") {
+           throw new ForbiddenError();
+       }
        return this.userModel.delete(id);
    }
}

So many possibilities... This is the problem with the controller and the pattern in general. It makes things too ambiguous and in some cases, it spreads business logic everywhere. The worst is even codebases where some checks are made in the middleware, others in the controller, others in the service once we have fetched the resource to check ACLs against, etc...

Testing

The more responsibility the controllers have, the riskier and fragile the codebase is.

Indeed, testing controllers is cumbersome as we often need to start the whole server and make actual HTTP calls. Some frameworks provide solutions to this problem (e.g. Spring) but it is still cumbersome and makes tests taking more time to run.

Plus, even with AI, writing and maintaining such tests is very annoying as they tend to be very verbose.

Taking a step back

Looking at the bigger picture, we can see a clear pattern for each business feature :

• An input
• An output
• A behavior
• A permission

In the controller, we have to :

• Read the input from c.req.json, c.req.params, c.req.query manually
• Call the business feature manually, sometimes named the same as the controller method, sometimes not
• Map the response to send manually
• Map the error codes manually
• etc.

Whoever has worked on a professional MVC application has seen the huge quantity of code present in the controller layer. Even with AI, this causes a huge technical debt for companies, making things very hard to maintain.

Instead of wiring all of this manually and producing so much boilerplate, can't we have an orchestration layer that does the work for us ? Can't we imagine an orchestrator provided by the framework that automatically maps each service method to its endpoint ? No more controllers/*, no more routes, no more params extracted manually, no more responses built manually...

For that, we need a way to declare our business features in an explicit way, with the proper typing, that can be orchestrated to determine how to read the input, what behavior to call, what output to send, etc.

Guess what ? In libmodulor there is no controller layer. Each business case is declarative, via the UseCase mechanism.

We never have to write a controller, an endpoint or a route, just the business behavior that is automatically "mounted" and "exposed". It is also automatically tested on a real server, without writing anything, and it is blazingly fast.

No boilerplate, just the required declarative parts, agnostic to the tech exposing them (express, hono, fastify, etc.) to make things work.

Give it a try. Check out the new v0.32.0 bringing lots of improvements, especially in the testing feature.