Are you overloading your Controller?

Whenever I perform a Code Review or get involved in Pair Programming the most annoying behavior is when I see hundreds of lines of code in a single method in a controller and I'd be like…

“Why did you put every f*king thing inside the controller?”

MVC is a native coding design pattern built into most frameworks in many languages, so you know what to do inside a Model, View and Controller. Here's an explanation in a nutshell:

A model stores data that is retrieved according to commands from the controller and displayed in the view.
A view generates new output to the user based on changes in the model.
A controller can send commands to the model to update the model’s state (e.g., editing a document). It can also send commands to its associated view to change the view’s presentation of the model (e.g., scrolling through a document, movement of document)
Source: wiki

So it means if the controller is the center of everything, it might contain a bunch of logic like validating data before it's passed to a Model or even a form result to send it to View, so you will not be surprised if you see thousands of lines of code in single controller file BUT that’s nightmare to the next guy who are going to take care of it or even you, right?

Let’s imagine we are building a web service to provide some sort of data that require users to submit their information. Once the user submits their information through the View, here is what the controller does:

• Sanitize data: Trim all input strings, convert every empty input into NULL.
• Validate data: Making sure all inputs fields have been filled, email must be typed in the correct format, password must be at least 8 characters and must be confirmed and so on…
• Communicating with DB: Create a new record or update it in the database.
• Responding: Return data in format that a requester expects.

See the code for the big picture

It always seem simple at first but after new requirements are added, the excuses probably starts with “These lines will not be used anywhere else so better put it here.” or “C’mon it’s just few lines of validation, it will not mess anything up.” and then it’s too late to moving on.

What I suggest is…

From my point of view: It’s true that the controller is the center of everything but the controller is supposed to control things by letting the other classes or services do stuff and not do everything by itself. So we should separate the logic into a unique class which only responds to one job like Laravel does.

Middleware for a sanitizing request

Laravel provides some middleware by default likeRedirectIfAuthenticated for redirecting any user who already has signed in to any page you defined orVerifyCsrfToken that throw's an exception if the user tries to do a non-read request (ex: POST, PUT or DELETE) with out a CSRF token.

And for trimming input you will find 2 middleware in Laravel 5.4+ (for example TrimStrings and ConvertEmptyStringsToNull) which do exactly what the class name says. (I have written a blog about this)

Validating data requests

Assuming that every request points to single method in a controller and it’s inputs need to be validated, we are talking about Form Request. I personally like this kind of mechanism because I have been discussing with my team “Where do we put the validation? In the model or in the controller?” and this is the solution. I can finally sleep with my eyes closed now.

Communicating with DB

It’s not wrong that you are using SQL or the Eloquent Model in the controller but in a code pattern designer’s world we have many ways to improve this and one of the famous ones is Repository Pattern that will wrap your model or complicated SQL with a particular method like findByName or createIfNotExists — anything you define. The benefit of using it is flexibility, re-usable, clean and readable.

Responding

You may face a situation that you stored a date of birth in the database and you need to transform it into an age before sending it to the View. It won't take many lines of code to calculate it but it's better to have it somewhere else like the Transformer. There are many existing library outside or you just create your own, up to you!

And in the end your controller will look simple…

What do you think? This is the important practice that I always strict when I perform Code Review and Pair Programming because it leads to good behavior of coding.

Reference:

• Middleware: https://laravel.com/docs/5.5/middleware
• Form Request Validation: https://laravel.com/docs/5.5/validation#form-request-validation
• Repository Pattern: http://designpatternsphp.readthedocs.io/en/latest/More/Repository/README.html
• Repository Pattern: https://stackoverflow.com/questions/16176990/proper-repository-pattern-design-in-php