In MVC usually the controller sets whatever needs to be sent back to the client/View, including HTTP status code, e.g.:
class Controller {
public function get(Request req, Response resp) {
if (this.model.get(req.getId()) {
resp.setStatusCode(Response::HTTP_OK)
} else {
resp.setStatusCode(Response::HTTP_NOT_FOUND)
}
}
}
There are a few problems in doing it this way:
- all controller methods become littered with if/else for setting different HTTP status code
- I’ve encountered a problem where someone has a Model::createOrGet(), which just return Model. The controller doesn’t know if one has been created or fetch from store.
- setting HTTP status code code becomes duplicated between controllers.
Would it be better to make Model pass HTTP status code back to the controller, e.g.:
class Controller {
public function get(Request req, Response resp) {
cart := this.model.getOrCreate(req.getCustomerId()
resp.setStatusCode(this.model.getStatusCode())
if (cart) {
return cart
}
}
public function create(Request req, Response resp) {
cart := this.model.getOrCreate(req.getCustomerId()
resp.setStatusCode(this.model.getStatusCode())
if (cart) {
return cart
}
}
public function put(Customer c, array data) {
// this can result different success/error status codes
this.model.processPutDataForCustomer(c, data)
resp.setStatusCode(this.model.getStatusCode())
}
}
Basically this.model sets the last status to the appropriate HTTP status code which can be one of the success, client error or server error codes and the controller can just get that status code.
Is this good or is there a better way for this common problem? Isn’t it logical for web application to know HTTP status code deeper than the controller?
I’d really like to see example for multiple possible HTTP status code to be returned by the controller, something like a PUT request that can be one of “Bad Request”, “Conflict”, “Not Found”, “Created” and “No Content”.
The Controller is responsible for handling the HTTP request and knowing what to do. Setting the HTTP status code in the Controller is the right place to do this.
There are a few problems in doing it this way:
- all controller methods become littered with if/else for setting different HTTP status code
That’s where this code was meant to go.
- I’ve encountered a problem where someone has a Model::createOrGet(), which just return Model. The controller doesn’t know if one has been created or fetch from store.
If the Controller needs to know whether or not something was created, then the Controller shouldn’t be calling Model::getOrCreate(...).
- setting HTTP status code code becomes duplicated between controllers.
If the model set the HTTP status code then it’s duplicated between models, so this doesn’t really buy you anything either.
This next suggestion will largely depend on the framework you are using, but most frameworks default to a 200 OK response, leaving you to just deal with the negative cases (404 NOT FOUND, etc).
But again, this is precisely what the Controller is meant to do.
Yes, it seems like you are duplicating code, but you are not duplicating business logic. That’s the kind of duplication you want to avoid. Duplicating HTTP status codes isn’t really what I’d call “duplication.” It’s part of the Controller’s job.
imel96 commented:
I know setting some status code is Controller’s job. But a lot of HTTP status codes are part of business logic and business logic should go in Model, right?
HTTP status codes are only a part of Business Logic if the application you are writing is a web server. If not, an HTTP status code is an artifact of your application being implemented as a web application. Imagine if you needed to take your business logic and reuse it in the context of an installed GUI application. Do HTTP status codes make sense then? They don’t. An HTTP status code is not business logic. It is application flow control. The Controller is responsible for flow control.
Business Logic is Why and your Controller should know What to do when something happens. Controllers don’t know Why something happens, because that’s the responsibility of Business Logic.
I also find some Model throwing exceptions with HTTP status code in it and then Controller copy that to Response object. That also indicates to me that Model really wants to return HTTP status code, and without exception (like when using Go), it’d be just like returning status code.
If the model is throwing exceptions containing HTTP status codes, the model was built wrong. An exception is an exceptional condition that cannot be handled by the code being called, and so must be handled by the caller. Validations, as a separate entity from Models (although validators should be calling methods on Models while performing validation) and Controllers should take care of checking the data before exceptions can be thrown by the model or data access layer
When using many status codes, isn’t it too much of business logic to be put in Controller?
No. Again, HTTP status codes have nothing to do with Business Logic. The very fact that you’ve asked this question should show you how confusing it becomes when you mix up two completely different concerns: Business Logic (Models) and Flow Control (Controllers).
5
The controller’s job is to mediate between the model and the view. It tells the model what, in the abstract sense, the user wants it to do and tells the view how it should look based on the state of the model. Importantly, it is not the controller’s job to decide what the state of the model should be.
The server call and the data it returns are parts of the model and should therefore be handled by the model. The controller should not even tell the model to make a server request, much less directly observe the result of that request.
In a well designed system, the controller should not know where the data is coming from or what the model must do to get it. The model should be free to change its source of truth, for example from the server to a cache, without any changes in the controller.
In essence, I am saying that function get(Request req, Response rest) shouldn’t be in the controller in the first place. Instead there should be some sort of “userWantsX” function in the model and how the model responds to that request for X is it’s own business.
So the controller’s job is to convert UI gestures (user tapped button X) into model requests, (user wants X). Then after the model changes state, to convert that state information into UI view state changes. That’s all the controller should be doing.
1
I recommend you to use proper error handling and/or validation system for mitigating code duplication and violation of single responsibility principle.
Every MVC framework has some central error management hooks like these on ASP.NET 6. All you need to do is throw the appropriate exception, catch back on your central error handling mechanism, finally set the appropriate status code to the response.
Your controller shouldn’t do the job of validation and error handling.