Jul 01 2009

Use specific return types in your ASP.NET MVC action methods

Category: Uncategorizedbengtbe @ 19:00

When looking at ASP.NET MVC examples on the web almost all action methods return ActionResult, even methods that could return a specific subclass. Here is an example from the NerdDinner source code:

public ActionResult Index(int? page)

{

    const int pageSize = 25;

    var upcoming = dinnerRepository.FindUpcomingDinners();

    var paginated = new PaginatedList<Dinner>(upcoming, page ?? 0, pageSize);

    return View(paginated);

}

This method always returns a ViewResult, still they declare the return type as the ActionResult base class. This practice often leads to test code like the following:

ViewResult result = controller.Index(0) as ViewResult;

Assert.IsNotNull(result, “Controller should return a ViewResult”);

In NerdDinner they even have a whole test just to make sure that the result is a ViewResult:

[TestMethod]

public void IndexAction_Should_Return_View()

{

    // Arrange

    var controller = CreateDinnersControllerAs(“robcon”);

    // Act

    var result = controller.Index(0);

    // Assert

    Assert.IsInstanceOfType(result, typeof(ViewResult));

}

All this testing code are unnecessary, if you just let your action methods use specific return types, like shown below:

public ViewResult Index(int? page)

{

    const int pageSize = 25;

    var upcoming = dinnerRepository.FindUpcomingDinners();

    var paginated = new PaginatedList<Dinner>(upcoming, page ?? 0, pageSize);

    return View(paginated);

}

This also makes it immediately clear for people reading the code, what this action method returns.

The only time I would return ActionResult is when different paths of the action method returns different subclasses. In these cases, you should of course write tests that verify the return types of the different paths.

In the book Pro ASP.NET MVC Framework Steven Sanderson also recommends returning specific types. Take a look at the quote below:

“This action method specifically declares that it returns an
instance of ViewResult. It would work just the same if instead the
method return type was ActionResult (the base class for all action
results). In fact, some ASP.NET MVC programmers declare all their
action methods as returning a nonspecific ActionResult, even if they
know for sure that it will always return one particular subclass.
However, it’s a well-established principle in object-oriented
programming that methods should return the most specific type they can
(as well as accepting the most general parameter types they can).
Following this principle maximizes convenience and flexibility for code
that calls your method, such as your unit tests.”

BTW: This blog post is not meant to criticize the NerdDinner tutorial. It is a great free tutorial, and is probably one of the best ways to get introduced to the ASP.NET MVC framework :)

kick it on DotNetKicks.com Shout it

Tags:

3 Responses to “Use specific return types in your ASP.NET MVC action methods”

  1. Rob Conery says:

    Actually this isn’t a very good idea because you can’t follow it consistently. Consider handling posts from forms – the general pattern is that if the POST works and the operation is completed you return a RedirectToAction result; if not you return a view. This could happen quite a few times.

    I would say for Index/Details actions – this could be more readable – but typing a return is akin to "coupling" if you will and if there was an "IActionResult" I might offer you should *always* use that for the same reasons people use interfaces in programming: Future Proofing.

    You don’t know that your method will always return a ViewResult – you could change it in the future to return Json (or something cool that MVCContrib is using, or maybe a result type you just created) and boom, you’ve broken your app.

    Code boundaries should be nimble, not strongly typed for the sake of readability :) .

  2. Travis says:

    What would be cool, is if I could just return the view model.

  3. bengtbe says:

    Hi Rob.

    Always nice to get comments from someone who I have following in Google Reader. I also appreciate your work on SubSonic and MVC-StoreFront/Kona. However this time I don’t agree with you :)

    Actually this isn’t a very good idea because you can’t follow it consistently. Consider handling posts from forms – the general pattern is that if the POST works and the operation is completed you return a RedirectToAction result; if not you return a view. This could happen quite a few times.

    I briefly talked about this in the post. In these cases I would return an ActionResult, and write tests to verify the return type of each of these paths.

    In my opinion, it is just as consistent to return the most specific ActionResult than to always return the base class. If you take this to far you can have all methods return object :)

    I would say for Index/Details actions – this could be more readable – but typing a return is akin to "coupling" if you will and if there was an "IActionResult" I might offer you should *always* use that for the same reasons people use interfaces in programming: Future Proofing.

    I can’t see that there are benefits with decoupling the controller from the different ActionResult subclasses. They are the return types of the actions, not dependencies that I need to break in order to test. Taking decoupling too far can even sometimes be an anti-pattern, e.g. the Entity interface anti-pattern.

    As Steven Sanderson writes "it’s a well-established principle in object-oriented programming that methods should return the most specific type they can (as well as accepting the most general parameter types they can)".

    You don’t know that your method will always return a ViewResult – you could change it in the future to return Json (or something cool that MVCContrib is using, or maybe a result type you just created) and boom, you’ve broken your app.

    Not at all. ReSharper will instantly tell me that the return type is wrong, and even suggest a fix for me. It will also instantly find all the test that has to be updated. Visual Studio will of course also give this feedback when compiling.

    By returning an ActionResult, I would first get feedback that something is wrong when all the tests for the action method fails. Depending on how good you are at writing tests you then have to figure out what is wrong. In the NerdDinner code many of the test will then fail because of a NullReferenceException.

    Code boundaries should be nimble, not strongly typed for the sake of readability

    When my code is easily testable (decoupled at the right places), I must say that I regard code readability higher than both decoupling and future proofing.

    Thanks for starting this interesting discussion :)

Leave a Reply