- Posted by liammclennan on April 24, 2009
Critics of ASP.NET MVC often point out that view code can end up looking a bit like classic ASP spaghetti code. Two of the major offenders are conditionals and loops in the view. Consider the following view code:
<script type="text/javascript">
jQuery(document).ready(function() {
<% if (Model.ShouldJumpToSection)
{ %>
Chapter.JumpToAnchor('<%= Model.Section.Slug %>');
<%
}
%>
});
</script>
It is difficult to read. A good strategy to clean up views is to try to move conditionals and loops to the controller or to helper methods. Rob Conery has tackled this on his blog. For situations like the above I like to use a helper extension method to move the conditional to a helper. The code is then simplified to:
<script type="text/javascript">
jQuery(document).ready(function() {
<% Html.WriteIf(Model.ShouldJumpToSection, "Chapter.JumpToAnchor('" + Model.Section.Slug + "');"); %>
});
</script>
You can see that it is much easier to read. Because this code is not using the <%= %> notation you have to remember to add a ; at the end of the line. An important thing to keep in mind is that the method paramters are evaluated before the method is called, meaning that they are evaluated even if the conditional is false. Here is the code for the helper methods that I use:
public static class HtmlExtensions
{
public static void WriteIf(this HtmlHelper helper, bool condition, string truePart, string falsePart)
{
helper.ViewContext.HttpContext.Response.Write(condition ? truePart : falsePart);
}
public static void WriteIf(this HtmlHelper helper, bool condition, string truePart)
{
WriteIf(helper, condition, truePart, string.Empty);
}
}
Another common pattern that I see in MVC views is:
<% if (ViewData.Model.Count == 0) { %>
Nothing to display
<% } else {
foreach (var item in ViewData.Model) { %>
...render the item here...
<% }
} %>
It is not too bad but it can be cleaned up a lot by moving the conditional to the controller and placing the body of the loop in an MVC View User Control. In the controller check if the collection is empty and if so render a special empty view. Then the view above can be simplified to:
<% foreach (var item in ViewData.Model) {
Html.RenderPartial("_Item", item);
} %>
No spaghetti here.