The problem with ASP.NET MVC - "spaghetti code"

Posted by Martyn at 4/9/2009 5:59:09 PM

Tags: MVC | ASP.NET

I apologise in advance if this post seems slightly incoherent - I write this whilst on a train, and its quite a bumpy ride.  Also, another disclaimer - the code you're about to see will be soon heavily refactored and improved; don't expect it in this state to be amazing!  For one, truncating the post content will be moved to the model level, and the post mark-up will be moved to a user control.

Consider the following mark-up:

<% foreach (Blog42.Model.Post p in Model.Posts) { %>

<%-- Post begin --%>

<div class="leftcontent">
    <h1><%= Html.ActionLink(p.Title,"Show", new { title = p.URI }) %></h1>
    <h2>Posted by <%= Html.ActionLink(p.User.UserName,"ByUser", new { username = p.User.UserName }) %>

     at <%= p.Date %></h2>
    
    <div class="blogcontent">
        <%= p.TruncatedContent %>
        <% if (p.IsTruncated) { %>
            <%= Html.ActionLink("Read More","Show", new { title = p.URI }) %>

        <% } %>
    </div>
                    
    <%= Html.RenderTags(p) %>
    
    <span><%= Html.ActionLink("Comments (" + p.Comments.Count + ")","Show", new { title = p.URI }) %></span>

</div>

<%-- Post end --%>

<% } %>

There are a couple of things here that I absolutely hate (and will soon in part be refactored) - the selection statement and the massive amount of <%= %>.  For most people, they will see nothing wrong with this, and will keep doing it.  In the end I believe this will lead to an un-maintainable application with unreadable mark-up.

Writing an helper is the solution to this - note that I have moved the tag building to a helper.

public static string RenderTags(this HtmlHelper helper, Post post)
{
    StringBuilder builder = new StringBuilder();

    if (post.Tags.Count >= 1)
    {
        builder.Append("<p><strong>Tags:</strong>");

        int tagNumber = 0;

        foreach (Tag tag in post.Tags)
        {
            tagNumber++;

            builder.Append(" ");
            builder.Append(helper.ActionLink(tag.Name,"ByTag","Post",new { tag = tag.Name }, null));

            if (tagNumber != post.Tags.Count)
                builder.Append(" | ");
        }

        builder.Append("</p>");
    }

    return builder.ToString();
}

This however, is also what I believe to be suboptimal - note the snippets of mark-up inside the code! 

However, it is an improvement, and for the time being I believe a necessary evil.  It does improve the readability of the mark-up, and introduces some semblance of reusability in the sense that I can use the same helper whenever I want to build a list of tags.

Lambda42 © Copyright 2010. All Rights Reserved.