Avoid MVC Anti Patterns
Friday, 30th July 2010
As part of my ongoing crusade to get ASP.NET MVC used properly, I have compiled a short list of things to avoid when using it. Most of these things are done by accident, but if you aren't careful they make it much harder to maintain your applications.
Follow the conventions
By default, your code structure will look like this.
URL:
http://mvc-application/Customer/Edit
Controller:
Controllers/CustomerController.cs
Method:
CustomerController.Edit()
View:
Views/Customer/Edit.aspx or Views/Shared/Edit.aspx where the view is genuinely share-able!
You should always try to stick to this convention.
If you find yourself writing the following line of code, please think twice:
return View("Edit", model)
This is typically done when someone decides that the "Edit" view does what they want, so instead of creating another view for their "Create" action, they just pass the create action into the "Edit" view. Instead of doing this, create a view with the correct name and convert the re-usable part of the "Edit" view into a partial view, which you can display in both the "Edit" and "Create" views. If you follow this pattern, your code should always look like this:
return View(model)
Sensible URLs
Keep your URLs sensible and meaningful. This is a good set of URLs for a simple application:
http://mvc-application/Customer/List
A list of customers
http://mvc-application/Customer/Detail/1
Displays the full detail of a given customer
http://mvc-application/Customer/Add
A form to add a new customer
http://mvc-application/Customer/Edit/1
A form to edit an existing customer (in this case, it will edit the customer with an id of 1)
http://mvc-application/Customer/Delete/1
Deletes a customer (and maybe displays a confirmation prompt... are you sure?!)
You should find that this extends as your application grows - add a new controller for each different entity, Customer, Product, Article and so on, and have a List, Detail, Add, Edit, Delete convention for the views. You can use a different convention, but keep it consistent within your application.
Always POST to the same place
Whenever you display a form, always POST the form to the same address. For example, when you display your "Add Customer" form:
http://mvc-application/Customer/Add
The form should POST to the same address (http://mvc-application/Customer/Add). In your controller, you should have the following two methods.
[HttpGet]
public ActionResult Add() {
...
}
[HttpPost]
public ActionResult Add(CustomerModel model) {
...
}
And if you follow this advice, you will never have to use anything more complicated than this to wrap a form on your view:
using (Html.BeginForm()) {
...
}
In this example, note that we just let it do it's stuff, we don't tell it to post to a different controller or action, it all just works. This tells us that this is how it was designed to work in the first place!
Using MVC as it was intended is not a constraint, it actually guides you into doing things the best way.
To sum up, here are some MVC-naughties that I would suggest you avoid.
- In an action, never render a view that belongs to a different action, such as showing the "Edit" view from the "Add" page. (Remember, you can avoid duplication by taking the shared part of the view and creating a partial view that can be re-used in both the "Edit" and "Add" views.
- Never ask a form at one address to post to another address. One good reason for this is that if validation fails, you need to re-display the form - which may result in spaghetti view references in your controller.
- Try to avoid specifying controllers and actions anywhere. Follow the MVC naming and folder structure conventions and it will all match up automatically.
- Keep your controllers specific. Don't keep adding more and more actions to your "HomeController". Try to have controllers that service a logical grouping of information, such as "CustomerController", "ProductController" and so on.