MVC Best Practices

Simone has a great post (as usual) on 12 ASP.NET MVC Best Practices:

Controller:

1 – Delete the AccountController

2 – Isolate controllers from the outside world

3 – Use an IoC Container

4 – So NO to “magic strings”

5 – Build your own personal conventions

6 – Pay attention to the verbs

Model:

7 – Domain Model != ViewModel

8 – Use ActionFilters for “shared” data

View:

9 – NEVER use code-behind

10 – Write HTML each time you can

11 – If there is an if, write an HtmlHelper extension

12 – Choose your ViewEngine carefully

This is a fantastic list, and of course I have my own additions/suggestions :)   First, I’m not a fan of action filters for shared data.  If we’re doing strongly-typed views, using action filters for shared data puts us back into the magic string land, where we have to either use inheritance in our ViewModels, or the dictionary part of ViewData.  I don’t like either approach, I’d rather go with something like RenderAction to truly enforce SRP in my ViewModels.  Not everyone appreciates the elegance of this approach, but I’m sure they’ll agree eventually.

I also don’t like writing an HtmlHelper extension just because I have logic in the view.  For a half-decent view engine, it shouldn’t be that problematic to have view logic in the view.  HtmlHelper extensions enforce the Helper object anti-pattern – a bunch of procedural logic hanging off one static class.  Instead, we went the route of building intelligent input builders for input elements, and only really using HtmlHelper when we want to eliminate duplication between multiple views.  I see these constructs:

  • HtmlHelper
  • Partials
  • Master Pages

All as means of eliminating duplication in our views, but not much more.  If there’s logic in a view, that’s fine by me as long as it’s not duplicated.  It’s not my fault that C# looks downright bizarre mixed in with HTML markup, but that’s what other view engines are for.

Finally, if there was an award for How Not to Design a Controller, the AccountController would be the runaway champ.  I still can’t imagine why it’s necessary, and the idea of portable areas would be a better fit for its functionality than the Thing We Always Delete with Extreme Prejudice.

Related Articles:

Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Jimmy Bogard

I'm a technical architect with Headspring in Austin, TX. I focus on DDD, distributed systems, and any other acronym-centric design/architecture/methodology. I created AutoMapper and am a co-author of the ASP.NET MVC in Action books.
This entry was posted in ASP.NET MVC. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • gc

    I agree on the RenderAction approach, I’d much rather have a separate internal request to render widgets such as the currently logged in user’s status in the header etc.

    But what if all the views should check on the current user’s permissions in order to render or not render some of their content. How do you handle logic like that?

    Currently I have all my ViewModels inherit from a base ViewModel which has a property that can be checked for the user’s permissions and is being populated on every request by a custom Controller Action Invoker, but so far it has only given me headache and it’s much too fragile and harder to test (specially when a Viewmodel contains list of child Viewmodels and they need to have those data populated too)

    What’s your approach to situations like this?

  • Ryan Roberts

    I use simple behaviours to deal with common elements on view models (http://pastie.org/673466). They are executed after the controller action using a common base controller.

    public class LoginBehavior : IBehavior
    {
    public void Apply(BaseViewModel sourceModel)
    {
    sourceModel.Login.CurrentMember = MemberContext.CurrentMember
    .Present();
    }
    }

    You end up with pretty good SOC for dealing with bits of the view model that are not directly the responsibility of the controller action.

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @gc

    We’ll still go RenderAction route, but we have a RequiresPermission attribute that’ll render nothing if the user doesn’t have permission. This was kinda tailored to our requirements, so it might not fit your situation.

    @Ryan

    That’s a pretty cool concept – we do something a lot more basic and hard-code (i.e., interface called IRequireInitialization :P ) But I still don’t like inheritance/composition in the view model, I just get annoyed with orthogonal concerns represented in one object, and with a mapping operation going on too, it might not work with how we set up our pipeline. I can definitely see the value of doing this approach though.

  • Ryan Roberts

    I think inherited view models may break down unpleasantly if/when or if they do not match the visual inheritance of the site. For the pretty simple things I have been using it for it has been fine so far.

  • http://channel9.msdn.com Mike Sampson (Sampy)

    The idea of how to send the result of composed “controller” actions to the View layer is something I spend a lot of time thinking about and grappling with. By “controller” I mean all the pieces that do work in the controller layer for the view: the actual Action method, filters, defaut values, etc.

    I think of the ViewData object as a box that I’m handing to the View that has everything it needs to get it’s job done. Inside the box are lots of other, smaller boxes. Each piece of the View layer (the actual View, partial views, master page) all know what they’re looking for in the box based on it’s type. Each one gets what it needs with out asking for it via it’s .Model property. Just declare your ViewModel type and you’ll get what you need automatically. You build this on the ViewData dictionary.

    Hating the ViewData dictionary with it’s “magic strings” is pretty common but I think that it is wrong to throw it out the window completely. You shouldn’t be playing in it at a low level but building code on top of it that makes your access to the contents strongly typed can enable some really good composition scenarios. Use type names as keys and build some glue in your view engine and you can have great, declartive composition in your View layer just like in your Controller layer.

    While ViewData["MyKey"] is pretty fragile, that doesn’t mean touching the dictionary is wrong.

  • EricTN

    Could you please provide a simple sample MVC2 application that has a simple replacement for the functionality provided by the Account Controler that you state should be immediately deleted. Thanks!

  • http://codeclimber.net.nz Simone Chiaretta

    @Jimmy, thank you commenting with a blog post :)
    Just making a few comments on the two things we kind of disagree on:

    1 – actionfilters: what if the magic strings were not exposed to the developers but “hidden” behind a “widget” library?

    2 – HtmlHelpers: yeah, I know, the helper pattern sucks, but I guess it’s better than sticking complex conditions mixed with HTML. That’s why the following best practice was to “choose your viewengine carefully” :)
    Another option, which I prefer, is trying to avoid all conditions in the view, and have these conditions already evaluated in the ViewModel. And have the view only handle plain boolean values.

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @EricTN

    Do you want another controller that does login/logout functionality? We have our own way of doing it, almost unique per project…

    @Simone

    Yeah, we’ve started doing more of the “precalculated logic” as well, it’s turning out pretty nicely!

  • http://serialseb.blogspot.com Sebastien Lambla

    Jimmy,

    in the original presentation I created, I introduce the concept of PresentationModels, a secondary controller who’s responsibility is to encapsulate the display logic out of the views and that receives the model returned by the controller, which solves nicely the problem of testable presentation logic.

    You can see the original at http://developerdayscotland.com/sessions/09-SebastienLambla/mvc-bestpractices.pdf and http://serialseb.blogspot.com/2009/05/my-mvc-best-practices-talk.html

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @Seb

    Ah, just now saw that his presentation was based on yours…thanks for the link!