Anti-Patterns in ASP.NET MVC

1. Over-use of Html Extension Methods

Html extension methods can be a great way to write clean razor code and to avoid repeating the same pieces of markup e.g. for a custom input control, but it’s important to know when you’d be better off using another technique. If your Html extension methods are getting too big for comfort, consider the following:

1.1. Refactor to use DisplayTemplates or EditorTemplates (good for simple properties, or editable properties)

If you need to render specific markup for a model property consider using DisplayTemplates (or EditorTemplates if it is for some sort of input field). In your razor markup you would use:

@Html.DisplayFor(m => m.SomeProperty)

or

@Html.EditorFor(m => m.SomeProperty)

Then under <YourWebProject>\Views\Shared\DisplayTemplates (or EditorTemplates), create a cshtml rendering with a name corresponding to the type of the model property. So if the field ‘SomeProperty’ above is of the type ‘MyCustomClass’ the DisplayTemplate or EditorTemplate would be called MyCustomClass.cshtml.

1.2. Refactoring into a separate controller (good for data access)

If your extension methods perform data access, you will find that it is difficult to make use of dependency injection. In this case, it is better to use a controller. In your razor code you would write

@Html.Action(“Controller”, “Action”)

1.3. Refactoring into a separate partial view (good for re-using html markup)

If your extension method was simply constructing html using model data, you could refactor this into a separate rendering called with:

@Html.Partial(“PartialViewName”, Model.Something)

Depending on the model type of the rendering, you could either pass in the containing view’s Model, or just a property of the view (or nothing if you don’t need to pull in any external data).

2. The ‘Fat Controller’

Some signs that you may be following the ‘fat controller’ anti-pattern are:

• object mapping code in the controller
• data access code in the controller

The first issue can be resolved by employing an object mapper such as AutoMapper. This way you can define mappings between different object types and map from one type to another using a single line of code.

The second issue (data access code in the controller) can be solved by moving your data access code to a service. This service can be referenced by an interface and wired-up using a DI (dependency injection) framework such as Ninject. Your controller’s constructor includes an argument of the type of the interface and the DI framework handles the rest.

Ian Cooper has written a whole article about the fat controller anti-pattern.

3. Placing Business Logic in Views

This is a common code smell which leads to hard to read, ugly razor code. Take the following example:

@if (DateTime.Now.Subtract(Model.LastPaymentDate).Days > 30)
{
<p class=”warning”>Please pay your bill now</p>
}

Which can easily be refactored into

@if (Model.PaymentOverDue)
{
<p class=”warning”>Please pay your bill now</p>
}

4. Not encapsulating action parameters into an object

This applies mainly to POST requests. Take the following code:

public ActionResult CreateCar(string make, string name, double engineSize, decimal price, string bodyType)

The method parameters can easily be replaced with a custom type and rewritten as:

public ActionResult CreateCar(CarDetails instance)

ModelBinding in ASP.NET MVC automatically handles creating an instance of the CarDetails class from the submitted form fields.

Feedback?

This article is a work in progress which I will add to as I spot more bad practices. In the meantime, if you've seen any other anti-patterns yourself, feel free to leave a comment below or via twitter. I will publish any other relevant suggestions giving full credit.