ASP.NET MVC TIP – Keep your controllers and actions thin

When writing you controller actions, keep them short and sweet. Don’t add a lot of actions and keep a close eye on the length of each action.
Whenever possible, I try to make my controller actions look something like this.

public ActionResult DoSomething()
{
    MyModel model = MyService.GetModel();
	return View("MyView",model);
}
//or
public ActionResult DoSomethingWithAKey(int myKey)
{
	MyModel model = MyService.GetModel(myKey);
	return View("MyView",model);
}

(1)

There are two reasons to do this:

1) It makes your controllers less complicated and easier to maintain. – You don’t want to have to do a lot of debugging in your controllers. Chances are, if you have a lot of complicated logic in your controller actions, you are probably mixing concerns. Make sure that your controllers are only responsible for getting data from your models and figuring out what view to send to the client.

2) It makes testing easier. If all you are doing is getting a model and passing that model to a view, what do you really need to test? Maybe that the correct view is being returned.

(1) Of course I use more descriptive names than “myKey”. :P

  • Derik Whittaker

    This should not be a tip, it should be a mandate

  • Glen

    If this is true, then the controller is actually just overhead code and should not be needed.

    Is there a way to do what these controllers are doing, by attributes or shared names?

  • http://wis3guy.net Geoffrey

    I agree totally to your suggestion, and actully have a very similar style to implement my controllers.

    Continuing on Glen’s statement, which imo does make sense, there should be an even cleaner way. I’d almost say something like events or delegates.

  • http://codeclimber.net.nz Simone

    Totally agree….
    There is a tenet in the RoR world that says:
    “Skinny controllers, Fat Model and… Dumb view”

  • http://blog.dynamicprogrammer.com Hernan Garcia

    Or we may risk to make the controller another code behind hell.

  • http://www.lazycoder.com Scott

    Glen: When I say “thin” read that as “as thin as possible”. You don’t want your controller to just turn into a routing rule. Sometimes you’ll need to put a little bit of logic in the controllers. I’ll cover one such case a little later in the week. :)

  • Matthew Pelser

    At some point you need to make multiple calls to the multiple models and then operate on these different result sets and return some sort of transformed output.

    Do you find that you end up with model classes calling other model classes for this output or is this another layer? Maybe the “MyService” layer in your example that ends up being a fat “model controller” that stitches and transforms the finer grained “Get” calls together.

    I totally see the value of having the controller class having the sole responsibility for attaching the model to the view, but you have to push the logic somewhere. The model layer feels like fine grained query layer to me not a “Business” (yuk) layer.

  • http://thoughtplex.com Tommy Kelly

    Matthew, I am using a service layer that transforms my business objects into DTO’s for my controllers. This makes less code in the controllers, where its somewhat harder to test. I find it works well for my project.

  • rokey

    I’m having my controller do the eager loading (since im using EF for model), and the error checking in the POST controllers…

    Does that mean a gigantic model class that provides different methods for loading same entity object with different combinations of relations?

  • C.T.

    What I used in several real websites is like this:
    a project inclues DAL, BLL and BO
    the other project is a website project having
    Controller
    View
    etc.

    i feel many times, a contoller need data/objects from different BLLs. and in some *controllers, i have to organize different data/objects, especially in POST *controllers, i have to write many codes to validate data.

    this made me headache, i want to thin the *controllers, but have not found a way to seperate the logic in the controllers.

    if it is possible that move most CONTROLLER logic to the MODEL layer?