[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.
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.
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" />
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.
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.
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.
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>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]
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.
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.
I do not want to repeat what Justin Etheredge has mentioned in his post, be careful and do not fall into that trap.
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.
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.
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.
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.