Wednesday, April 1, 2009 10:44 PM Kazi Manzur Rashid

ASP.NET MVC Best Practices (Part 1)

[Also check out the next part of this series]

In this post, I will share some of the best practices/guideline in developing ASP.NET MVC applications which I have learned in the hard way. I will not tell you to use DI or Unit Test instead I will assume you are already doing it and you prefer craftsmanship over anything.

1. Create Extension methods of UrlHelper to generate your url from Route

Avoid passing the controller, action or route name as string, create extension methods of UrlHelper which encapsulates it, for example:

public static class UrlHelperExtension
{
    public static string Home(this UrlHelper helper)
    {
        return helper.Content("~/");
    }

    public static string SignUp(this UrlHelper helper)
    {
        return helper.RouteUrl("Signup");
    }

    public static string Dashboard(this UrlHelper helper)
    {
        return Dashboard(helper, StoryListTab.Unread);
    }

    public static string Dashboard(this UrlHelper helper, StoryListTab tab)
    {
        return Dashboard(helper, tab, OrderBy.CreatedAtDescending, 1);
    }

    public static string Dashboard(this UrlHelper helper, StoryListTab tab, OrderBy orderBy, int page)
    {
        return helper.RouteUrl("Dashboard", new { tab = tab.ToString(), orderBy = orderBy.ToString(), page });
    }

    public static string Update(this UrlHelper helper)
    {
        return helper.RouteUrl("Update");
    }

    public static string Submit(this UrlHelper helper)
    {
        return helper.RouteUrl("Submit");
    }
}

Now, You can use the following in your view:

<a href="<%= Url.Dashboard() %>">Dashboard</a>
<a href="<%= Url.Profile() %>">Profile</a>

Instead of:

<%= Html.ActionLink("Dashboard", "Dashboard", "Story") %>
<a href="<%= Url.RouteUrl("Profile")%>">Profile</a>

And in Controller I can use:

return Redirect(Url.Dashboard(StoryListTab.Favorite, OrderBy.CreatedAtAscending, 1))

Instead of:

return RedirectToAction("Dashboard", "Story", new { tab = StoryListTab.Favorite, orderBy = OrderBy.CreatedAtAscending, page = 1 });

Of course you can use the strongly typed version which takes the controller, method and the parameters of the future assembly or create your own to avoid the future refactoring pain, but remember it is not officially supported and might change in the future. You can also use the above with the strongly typed version, certainly “adding another layer of indirection” (Scott Ha favorite quote) has some benefits. Another benefit when writing Unit Test you will only deal with the RedirectResult rather than RediretResult and RedirectToRouteResult.

2. Create Extension Method of UrlHelper to map your JavaScript, Stylesheet and Image Folder

By default ASP.NET MVC creates Content, Scripts folder for these things, which I do not like, Instead I like the following folder structure so that I can only set static file caching  on the Assets folder in IIS instead of going to multiple folders:

Assets
+images
+scripts
+stylesheets

No matter what the structure is, create some extension method of UrlHelper to map these folders, so that you can easily refer it in your view and later on if you need to change the structure, you do not have to do massive find/replace. I would also recommend to create extension methods for those assets which are often refereed in your views. For example:

public static string Image(this UrlHelper helper, string fileName)
{
    return helper.Content("~/assets/images/{0}".FormatWith(fileName));
}

public static string Stylesheet(this UrlHelper helper, string fileName)
{
    return helper.Content("~/assets/stylesheets/{0}".FormatWith(fileName));
}

public static string NoIcon(this UrlHelper helper)
{
    return Image(helper, "noIcon.png");
}

And when referring the assets you can use:

<link href="<%= Url.Stylesheet("site.css")%>" rel="stylesheet" type="text/css"/>

Instead of the default:

<link href="../../Content/Site.css" rel="stylesheet" type="text/css" />

3. Use Bootstrapper in Global.asax

I have already covered it in the past, basically if you are doing a lot things in Application_Start of global.asax e.g. Registering Routes, Registering Controller Factory, Model Binders, View Engine, starting application specific background services, create individual task for specific part, then do use Bootstrapper to execute those. This make your code lot more clean and testable. This will be also helpful when developing a portal kind of app in asp.net mvc where each module can have some startup initialization without affecting others. But if you are developing a small app where the above things will never be an issue you can surly go ahead with the default global.asax.

4. Do not make any hard dependency on the DI Container, use Common Service Locator

Do not clutter your code with any specific DI reference, instead use the Common Service Locator, it is an abstraction over the underlying DI and it has the support for all of the popular DI containers, so that you can replace the underlying DI without modifying your application code as each DI Container has some unique features over the others. Tim Barcz recently wrote a excellent post on this subject, how much obsessed we are with our favorite DI Container but I am not sure why he did not mention about it. The Common Service Locator has the support for most of the regular scenarios, but for specific case for example injecting dependency in already instantiated object which as per my knowledge StructureMap, Ninject and Unity supports you can call the static ServiceLocator.Current.GetInstance in the constructor instead of calling the underlying DI. And for those who do know, Common Service Locator is a joint effort of the DI Containers creators initiated by Jeremy D Miller.

Creating Controller Factory with Common Service Locator is very easy:

public class CommonServiceLocatorControllerFactory : DefaultControllerFactory
{
    protected override IController GetControllerInstance(Type controllerType)
    {
        return (controllerType == null) ? base.GetControllerInstance(controllerType) : ServiceLocator.Current.GetInstance(controllerType) as IController;
    }
}

I hope the MVCContrib guys will follow the same instead of creating separate Controller Factory for each Container.

5. Decorate your Action Methods with Proper AcceptVerbs Attribute

ASP.NET MVC is much more vulnerable comparing to Web Forms. Make sure the action methods that modifies the data only accepts HttpVerbs.Post. If security is too much concern use the ValidateAntiForgeryToken or you can use Captcha. Derik Whittaker has an excellent post as well as a screen cast on how to integrate reCaptcha with ASP.NET MVC application, which I highly recommend. (Side Note: Do not miss a single episode of DimeCasts.net, I have learnt a lot form those short screen casts). My rule of thumb is use HttpVerbs.Post for all data modification actions and HttpVerbs.Get for data reading operations.

6. Decorate your most frequent Action Methods with OutputCache Attribute

Use OutputCache attribute when you are returning the less frequent updated data, prime candidate may be your home page, feed etc etc. You can use it for both Html and Json data types. When using it, only specify the Cache Profile name, do not not specify any other thing, use the web.config output cache section to fine tune it. For example:

[AcceptVerbs(HttpVerbs.Get), OutputCache(CacheProfile = "Dashboard")]
public ActionResult Dashboard(string userName, StoryListTab tab, OrderBy orderBy, int? page)
{
}

And in web.config:

<system.web>
        <caching>
            <outputCacheSettings>
                <outputCacheProfiles>
                    <clear/>
                    <!-- 15 Seconds -->
                    <add
                        name="Dashboard"
                        duration="15"
                        varyByParam="*"
                        location="Client"
                    />
                </outputCacheProfiles>
            </outputCacheSettings>
        </caching>
</system.web>

7. Keep your controller free from HttpContext and its tail

Make sure your controller does not have to refer the HttpContext and its tail. it will make your life easier when unit testing your Controller. If you need to access anything from HttpContext like User, QueryString, Cookie etc use custom action filter or create some interface and wrapper and pass it in the constructor. For example, for the following Route:

_routes.MapRoute("Dashboard", "Dashboard/{tab}/{orderBy}/{page}", new { controller = "Story", action = "Dashboard", tab = StoryListTab.Unread.ToString(), orderBy = OrderBy.CreatedAtDescending.ToString(), page = 1 });

But the controller action methods is declared as:

[AcceptVerbs(HttpVerbs.Get), OutputCache(CacheProfile = "Dashboard"), UserNameFilter]
public ActionResult Dashboard(string userName, StoryListTab tab, OrderBy orderBy, int? page)
{
}

The UserNameFilter is responsible for passing the UserName:

public class UserNameFilter : ActionFilterAttribute
{
    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        const string Key = "userName";

        if (filterContext.ActionParameters.ContainsKey(Key))
        {
            if (filterContext.HttpContext.User.Identity.IsAuthenticated)
            {
                filterContext.ActionParameters[Key] = filterContext.HttpContext.User.Identity.Name;
            }
        }

        base.OnActionExecuting(filterContext);
    }
}

[Update: Make sure you have decorate either the Action or the Controller with Authorize attribute, check the comments]

8. Use Action Filter to Convert to compatible Action Methods parameters

Use Action Filter to convert incoming values to your controller action method parameters, again consider the Dashboard method, we are accepting tab and orderBy as Enum.

[AcceptVerbs(HttpVerbs.Get), OutputCache(CacheProfile = "Dashboard"), StoryListFilter]
public ActionResult Dashboard(string userName, StoryListTab tab, OrderBy orderBy, int? page)
{
}

The StoryListFilter will be responsible to convert it to proper data type from route values/querystrings.

public class StoryListFilter : ActionFilterAttribute
{
    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        const string TabKey = "tab";
        const string OrderByKey = "orderBy";

        NameValueCollection queryString = filterContext.HttpContext.Request.QueryString;

        StoryListTab tab =  string.IsNullOrEmpty(queryString[TabKey]) ?
                            filterContext.RouteData.Values[TabKey].ToString().ToEnum(StoryListTab.Unread) :
                            queryString[TabKey].ToEnum(StoryListTab.Unread);

        filterContext.ActionParameters[TabKey] = tab;

        OrderBy orderBy =   string.IsNullOrEmpty(queryString[OrderByKey]) ?
                            filterContext.RouteData.Values[OrderByKey].ToString().ToEnum(OrderBy.CreatedAtDescending) :
                            queryString[OrderByKey].ToEnum(OrderBy.CreatedAtDescending);

        filterContext.ActionParameters[OrderByKey] = orderBy;

        base.OnActionExecuting(filterContext);
    }
}

You can also use the custom Model Binder for the same purpose. In that case you will have to create two custom Model Binders for each Enum instead of one action filter. Another issue with the Model Binder is once it is registered for a type it will always come into action, but action filter can be selectively applied.

9. Action Filter Location

If you need the same action filter to all of your controller action methods,  put it in the controller rather than each action method. If you want to apply the same action filter to all of your controller create a base controller and inherit from that base controller, for example the story controller should be only used when user is logged in and we need to pass the current user name in its methods, also the StoryController should compress the data when returning:

[Authorize, UserNameFilter]
public class StoryController : BaseController
{
}

[CompressFilter]
public class BaseController : Controller
{
}

But if the inheritance hierarchy is going more than 2 /3 level deep, find another way to apply the filters. The latest Oxite code has some excellent technique applying filters dynamically which I highly recommend to check.

10. Use UpdateModel Carefully

I do not want to repeat what Justin Etheredge has mentioned in his post, be careful and do not fall into that trap.

11.Controller will not contain any Domain logic

Controller should be only responsible for:

  • Validating Input
  • Calling Model to prepare the view
  • Return the view or redirect to another action

If you are doing any other thing you are doing it in a wrong place, it is rather the Model responsibility which you are doing in Controller. If you follow this rule your action method will not be more than 20 – 25 lines of code. Ian Cooper has an excellent post Skinny Controller Fat Model, do read it.

12. Avoid ViewData, use ViewData.Model

Depending upon the dictionary key will not only make your code hard to refactor, also you will have to write the casting code in your view. It is completely okay even you end up per class for each action method of your controller.  If you think, creating these kind of classes is a tedious job, you can use the ViewDataExtensions of the MVCContrib project, it has some nice extension for returning strongly typed objects, though you still have to depend upon the string key if you have more than one data type in ViewData Dictionary.

13. Use PRG Pattern for Data Modification

Tim Barcz, Matt Hawley, Stephen Walther and even The Gu has blogged this over here, here, here and here. One of the issue with this pattern is when a validation fails or any  exception occurs you have to copy the ModelState into TempData. If you are doing it manually, please stop it, you can do this automatically with Action Filters, like the following:

[AcceptVerbs(HttpVerbs.Get), OutputCache(CacheProfile = "Dashboard"), StoryListFilter, ImportModelStateFromTempData]
public ActionResult Dashboard(string userName, StoryListTab tab, OrderBy orderBy, int? page)
{
	//Other Codes
    return View();
}

[AcceptVerbs(HttpVerbs.Post), ExportModelStateToTempData]
public ActionResult Submit(string userName, string url)
{
    if (ValidateSubmit(url))
    {
        try
        {
            _storyService.Submit(userName, url);
        }
        catch (Exception e)
        {
            ModelState.AddModelError(ModelStateException, e);
        }
    }

    return Redirect(Url.Dashboard());
}

And the Action Filers

public abstract class ModelStateTempDataTransfer : ActionFilterAttribute
{
    protected static readonly string Key = typeof(ModelStateTempDataTransfer).FullName;
}

public class ExportModelStateToTempData : ModelStateTempDataTransfer
{
    public override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        //Only export when ModelState is not valid
        if (!filterContext.Controller.ViewData.ModelState.IsValid)
        {
            //Export if we are redirecting
            if ((filterContext.Result is RedirectResult) || (filterContext.Result is RedirectToRouteResult))
            {
                filterContext.Controller.TempData[Key] = filterContext.Controller.ViewData.ModelState;
            }
        }

        base.OnActionExecuted(filterContext);
    }
}

public class ImportModelStateFromTempData : ModelStateTempDataTransfer
{
    public override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        ModelStateDictionary modelState = filterContext.Controller.TempData[Key] as ModelStateDictionary;

        if (modelState != null)
        {
            //Only Import if we are viewing
            if (filterContext.Result is ViewResult)
            {
                filterContext.Controller.ViewData.ModelState.Merge(modelState);
            }
            else
            {
                //Otherwise remove it.
                filterContext.Controller.TempData.Remove(Key);
            }
        }

        base.OnActionExecuted(filterContext);
    }
}

The MVCContrib project also has this feature but they are doing it in a single class which I do not like, I would like to have more control which method to export and which to import.

14. Create Layer Super Type for your ViewModel and Use Action Filter to populate common parts.

Create a layer super type for your view model classes and use action filter to populate common things into it . For example the tiny little application that I am developing I need to know the User Name and whether the User is authenticated.

public class ViewModel
{
    public bool IsUserAuthenticated
    {
        get;
        set;
    }

    public string UserName
    {
        get;
        set;
    }
}

and the action filter:

public class ViewModelUserFilter : ActionFilterAttribute
{
    public override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        ViewModel model;

        if (filterContext.Controller.ViewData.Model == null)
        {
            model = new ViewModel();
            filterContext.Controller.ViewData.Model = model;
        }
        else
        {
            model = filterContext.Controller.ViewData.Model as ViewModel;
        }

        if (model != null)
        {
            model.IsUserAuthenticated = filterContext.HttpContext.User.Identity.IsAuthenticated;

            if (model.IsUserAuthenticated)
            {
                model.UserName = filterContext.HttpContext.User.Identity.Name;
            }
        }

        base.OnActionExecuted(filterContext);
    }
}

As you can see that it not replacing the model, if it is previously set in the controller, rather it populates the common part if it finds it compatible. Other benefit is, the views that only depends the layer super type you can simply return View() instead of creating the model.

That's it for today, I will post rest of the items tomorrow.

Stay tuned.

Shout it
Filed under: , , ,

Comments

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 1:34 PM by Christian

really helpful article! thank you very much!

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 4:39 PM by andrexx

Great post. Thank you. Agree with all, except using CommonServiceLocator. I think CommonServiceLocator has bad design.

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 5:04 PM by Kazi Manzur Rashid

@Christian : Thanks.

@andrexx: Thanks, but why do you think CSL is a Bad design, what makes you sad about CSL?

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 6:44 PM by Dominic Pettifer

Awesome tips, thank you. I've been waiting for someone to post to some practive advice for MVC for a while.

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 10:30 PM by Kevin Pang

Isn't #7 a bit of a security risk?  What if someone were to go to that URL and pass in a url parameter of "userName"?  Wouldn't that take precedence over the "UserNameFilter" attribute and essentially allow them to masquerade as another user?

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 10:48 PM by Kevin H

Not to be pedantic, but is it Dahsboard, or Dashboard in #1?

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 11:13 PM by cvertex

i don't get the desire to decouple from DI container.

if the DI container works well in unit testing, then why bother decoupling.

This doesn't mean static references everywhere, but why can't your controller factory be DI container dependent?

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 1, 2009 11:20 PM by Kazi Manzur Rashid

@Kevin Pang : The Controller itself is decorated with Authorize attribute, no matter what values you provide it will be replaced with the currently logged in user.

@Kevin H : Sorry Typo, fixed, thanks for pointing it out.

# re: ASP.NET MVC Best Practices (Part 1)

Thursday, April 2, 2009 12:15 AM by Kevin Pang

@Manzu

Yes, in your case it should work.  However for anyone else that uses that code snippit, they will need to make sure that either an [Authorize] attribute is on the controller and/or action OR that they modify the code to replace the username with an empty string or some other value that will indicate the user isn't logged in as the username passed in the url.

# re: ASP.NET MVC Best Practices (Part 1)

Thursday, April 2, 2009 12:37 AM by Minhajuddin

Very neat article with lots of good tips

# re: ASP.NET MVC Best Practices (Part 1)

Thursday, April 2, 2009 4:25 AM by Hadi Hariri

Why would you prefer #1 over

<%=Html.ActionLink<HomeController>( a => a.Index())%>

?

# re: ASP.NET MVC Best Practices (Part 1)

Thursday, April 2, 2009 11:26 AM by Chad Moran

Fantastic post.  Lots of things I had in the back of my mind but never got around to doing.

# re: ASP.NET MVC Best Practices (Part 1)

Thursday, April 2, 2009 8:32 PM by Harry

Hey excellent tips here!  thanks for sharing with the rest of us

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 2:09 AM by dziedrius

i wanted to ask the same question as Hadi Hariri - why not to use Html.ActionLink<T> or Html.BuildUrlFromExpression<T> for #1 tip?

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 2:55 AM by Kazi Manzur Rashid

@cvertex:

Checkout the post of Tim Barcz, most of the people who uses DI are very much sensitive about their preferred DI container. Using CSL I am giving a chance to use their preferred DI instead of mine. In addition, if I find that a new or another DI is best match for my changing scenario, I can switch to that without having much more pain.

No, using CSL does not mean that you have to have static reference, even if you do, in some situation like action filter, it is much better than referencing the actual Container IMO.

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 2:56 AM by Kazi Manzur Rashid

@Kevin Pang: Updated

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 3:02 AM by Kazi Manzur Rashid

@Hadi and @dziedrius:

This is not

<%=Html.ActionLink<HomeController>( a => a.Index())%>

vs # 1

It is

<%=Html.ActionLink<StoryController>( c => c.Dashboard(new { tab = StoryListTab.Favorite, orderBy = OrderBy.CreatedAtAscending, page =10 }))%>

vs

Url.Dashboard(StoryListTab.Favorite, OrderBy.CreatedAtAscending, 1)

and in my opnion the later is much more readable and compact.

also the lambda version is not officailly supported.

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 3:16 AM by Kazi Manzur Rashid

@everyone: thanks for your comments.

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 12:10 PM by Augi

Really great stuff! I have only one note to #8: I think that it's better to use "filterContext.Controller.ValueProvider.GetValue" instead of "filterContext.HttpContext.Request.QueryString".

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 12:19 PM by reamon

Thanks for the PRG related tip. I was just starting down the path of figuring out how to do this in a simple, consistent way. The use of ActionFilter is a great approach. You saved me some time!

What I particularly liked was that, among the multiple blogs on the topic, you seem to be the only one that understood that a post handling action should *always* redirect.

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 1:17 PM by Luiz

I've Translated your post

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 3, 2009 2:21 PM by Corey Gaudin

I particularly like your use of Action Filters for PRG Pattern.

However, what if you would do something more direct instead of an attribute.

Such as adding the following Extension method to ModelState in your Post Action:

ModelState.StoreToTemp();

And adding the other Extension Method in your Get Action that you redirected to:

ModelState.RestoreFromTemp();

Either way works, but it seems that the Extension way of doing it is is more direct when it occurs (in other words you can direct when to load and store, where-as the Action Filter you can only do before or after the action has accorded.

# re: ASP.NET MVC Best Practices (Part 1)

Saturday, April 4, 2009 6:30 AM by munnacs

Good one ...

# re: ASP.NET MVC Best Practices (Part 1)

Sunday, April 5, 2009 5:42 AM by Sohan

Good comprehensive post. Liked it!

# re: ASP.NET MVC Best Practices (Part 1)

Sunday, April 5, 2009 5:43 AM by Sohan

Please let me know which tool are you using for posting code snippets? Your code blocks look good.

Please reply me personally at sohan39@gmail.com

# re: ASP.NET MVC Best Practices (Part 1)

Friday, April 10, 2009 1:33 AM by Ahasan Habib

Very will described and resourceful Article. The articles which are request to read by author of this article also very helpful. I learned many things from all this. Thanks!!!

# re: ASP.NET MVC Best Practices (Part 1)

Wednesday, April 15, 2009 8:34 AM by mogadanez

Insead of 2. (and for some more view-oriented practicies) I prefer use Spark View Engine. it support ~/ syntax for urls.more even it support something like

~/${Theme}/images/logo.gif