Every time I do a demo or post something about MVC, I invariably get the comment: "this is Spaghetti Code from the bad old days of ASP Classic". Gimme a break. Have we been handheld for so long that we forget how to do this stuff?
I recognize quickly that people see the <%= markup and, like cats with a spray bottle, instantly associate it with a Bad Time. I know I'll never stop hearing it, but I also won't give up insisting that you don't need to settle for bad markup in your view - you have the answer. You've had it all along...
Mr. Atwood's Wrecking Ball
The original title of this post was supposed to be a response to Jeff Atwood's Tag Soup post and I was going to title it "OMFG Atwood's Coding Again" but I decided against it - mainly because I wanted him to help me spec and build my new PC (and overclock it). I also figured that it would come off as a snarky-sounding post, in a retaliatory way and that's not the intention.
Jeff wrote what I call the "canonical OMG how do I use this shiny new gun of a platform" post. Perhaps it's a little negative - focusing on things like...
Look at how many ways they gave me to screw everything up!
...rather than what's intended, which is more of a "wow! look what I have control over!" This quote summarizes Jeff's issues:
"As we work with ASP.NET MVC on Stack Overflow, I find myself violently thrust back into the bad old days of tag soup that I remember from my tenure as a classic ASP developer in the late 90's. If you're not careful bordering on manically fastidious in constructing your Views, you'll end up with a giant mish-mash of HTML, Javascript, and server-side code. Classic tag soup; difficult to read, difficult to maintain."
I have a few issues with this statement. You don't need to be "manically" (maniacally?) fastidious to produce easily readable code that is 10 times simpler to maintain than a single Repeater.
I may even dare to suggest here that the magic of <asp: controls does not prohibit a single thing that Jeff is wary of here. Most of the time you just use Code Behind as a giant rug, under which you might sweep the Eventing cruft that you have to write (OnRowDataBound anyone?).
I'm getting off track. It's really tempting to pick Jeff's post apart but that's not my point. Yesterday when we were geeking out over PC hardware I told him that he's a rather large figure with a massive wrecking ball hung over his shoulder (his blog). Invariably he may turn around quickly, hearing the sound of progress breaking twigs in the distance, and his wrecking ball goes flying - taking out acres of forest all at once.
I want to keep the MVC forests well-greened. Moreover I want to answer Jeff's question:
...is there a better way? Is there something beyond RHTML, Views, and Templates? What examples would you point to of web development stacks that avoided degenerating into yet more hazardous, difficult to maintain tag soup? Is there anything truly better on the horizon?
Or is this year's newer, fancier, even-more-delicious iteration of tag soup as good as it ever gets for web development?
Understanding The Concern (and Then Separating It)
Jeff's main issue here is the "mingling of tags". The Typo example that he posted is a nasty one, to be sure (and perhaps "over-represents" his point) and perfectly represents what he's getting at: it's nearly impossible to tell what's HTML, what's server-side code, and what's javascript. In short it's a mess.
That will be my mandate today: to show you how you can take this mess and clearly separate it using some good practices. This doesn't confine itself to ASP.NET MVC - any platform can do this.
Our Patient: The Pageable Grid
It took me a minute, but I grokked what was going on Jeff's Typo example in fairly short order. I know Rails pretty well (I'm assuming Jeff doesn't) and I was able to read this code. This is our starting point - let's clean this mess up:
If you don't know Rails/Ruby and can't tell, this blob of code is actually pretty dang functional. Here's what I can glean from this:
- It's a grid of pages from a Typo Blog, from the Admin interface
- It's paged, with a "Prev" and "Next" link
- It allows you to delete posts/pages
- There is conditional row styling for alternating rows, plus a special style for inactive posts
I may have missed something here - but this is a good place to start.
First Pass: I'm Glad I'm Using MVC
My first thought here is that if you told me I needed to use a GridView, I'd cry. I won't hide the fact that I like MVC over WebForms; and it's specifically for this reason. Just setting up paging in a GridView (assuming you're NOT using DataSets... which you better not be :) would triple the code that you see here. It doesn't matter if it's in the CodeBehind - it's still UI code.
Truncating the post title would require a "RowDataBound" event, then some custom logic that would determine the style (that would most likely be buried in the CodeBehind and not centralized and reusable).
Finally, deleting the post would most likely happen in the CodeBehind as well, creating even more code.
At this point I'd like to offer that Jeff's statement about "manically fastidious" applies ten-fold in the WebForms case. It's much, MUCH easier to bury your logic in the CodeBehind - and that to me is not a Good Thing.
To be fair - and I know I'll get comments on this - yes, if you're a GridView Wizard using the SqlDataSource you could make this all happen on the page. I call this Control Gymnastics, and I like to avoid this kind of thing :). It reminds me of my daughters putting dresses on their kittens, and then making them play Princess in the Castle. To the girls, the kitties are the cutest princesses ever. I'm just waiting for the cats to scratch them and run away...
Step One: Create The Grid
For this example, I've created a quick ASP.NET MVC site, and added an AdminController and Index view so I can approximate what's going on here:
I also created a Model called "Page" with the same properties as the example:
public class Page
{
public string Title { get; set; }
public string Permalink { get; set; }
public DateTime Created { get; set; }
}
In the AdminController I have a single action, Index(), that's simply stubbing up the Model (creating fake data) and passing it down to the view. I'm going to create 50 posts using a for loop, so I can show how to page this stuff.
The first pass at creating a simple grid is pretty simple (as you'd expect). I'm not afraid of HTML per se, so I have at it with a table:
<h1>Posts</h1>
<table>
<tr>
<td>Post</td>
<td>Permalink</td>
<td>Created</td>
<td>Option</td>
</tr>
<%foreach (AtwoodPost.Models.Page p in ViewData.Model.Pages){ %>
<tr>
<td><%=p.Title %></td>
<td><%=p.Permalink %></td>
<td><%=p.Created.ToLongDateString() %></td>
<td><a href=#>Delete</a></td>
</tr>
<%} %>
</table>
And this produces what you'd think:
Step Two: Make It Pageable
Let's add some paging here. Let's assume for now that I've setup the Index() action to take an extra setting for page number, and that I'm using the cool and groovy PagedList<T> that ScottGu created. I don't mean to gloss over this stuff, but I want to stay focuses on UI code and keep this post under a million words.
This is where discipline needs to kick in. Normally one might just throw the code on the page to setup paging - but I'm going to play by a rule I have that really helps me:
If there's an IF, make a Helper
Any UI logic you have can almost always be made just a bit more generic and reusable. This rule applied 99% of the time whenever you see that you're writing an "<%=if...". Creating paging code falls under the same category.
I'm going to create a pager that I like - which is numeric without the previous/next (I don't like those). To do this, I'm going to add a class to my "Helpers" directory, and call it "ViewExtensions". You can call this "DataExtensions" or "GridExtensions" as well - but this sort of listing is pretty generic, and I'm sure I'll use this again (in fact I use it in every project so far - you'll see it in the Storefront soon). I'm going to make an Extension method for HtmlHelper - I find this the most readable:
/// <summary>
/// Creates a generic pager for any data source
/// </summary>
/// <param name="urlFormat">The link format, like /controller/method/{0}</param>
/// <param name="totalPages">The count of pages</param>
/// <param name="currentPage">The current page number (note that this isn't an index)</param>
/// <returns>System.String</returns>
public static string NumericPager(this HtmlHelper helper, string urlFormat,
int totalPages, int currentPage)
{
string linkFormat = "<a href=\"{0}\">{1}</a>";
bool isFirst = true;
StringBuilder sb = new StringBuilder();
for (int i = 1; i < totalPages + 1; i++)
{
if (!isFirst)
sb.Append(" | ");
string pageLink = i.ToString();
//Test for current page
if (currentPage != i)
{
sb.AppendFormat(linkFormat, string.Format(urlFormat, i), pageLink);
}
else
{
//denote if this isn't the current page
sb.AppendFormat("<b>{0}</b>", pageLink);
}
isFirst = false;
}
return sb.ToString();
}
Now as I always say - this code could probably be improved. In the past, readers have really added some great value to my stinky samples - so if you see something you don't like - lemme know!
Now, to add the pager to the code, it's one line (the way we like it):
<h1>Posts</h1>
<table cellpadding=5>
<tr>
<td>Post</td>
<td>Permalink</td>
<td>Created</td>
<td>Option</td>
</tr>
<%foreach (AtwoodPost.Models.Page p in ViewData.Model.Pages){ %>
<tr>
<td><%=p.Title %></td>
<td><%=p.Permalink %></td>
<td><%=p.Created.ToLongDateString() %></td>
<td><a href=#>Delete</a></td>
</tr>
<%} %>
<tr>
<td colspan=4><%=Html.NumericPager("/admin/index/{0}",ViewData.Model.TotalPages,ViewData.Model.CurrentPage) %></td>
</tr>
</table>
Note that I'm working with typed data in the View, which has properties telling me about total pages and current page etc.
Next up, let's add in some conditional formatting for alternating rows. To do this, I'll keep working with the ViewExtensions and make this very, very generic so it can format any odd/even thing. Let's assume, for now, that my CSS has some classes called "normal-row" and "alternate-row"; here's my first pass:
public static string GetRowClass(this HtmlHelper helper, int rowIndex)
{
return rowIndex % 2 == 0 ? "normal-row": "alternate-row";
}
This works, and I can pop this in my table with some small modifications:
<%int rowIndex = 0; %>
<%foreach(AtwoodPost.Models.Page p in ViewData.Model.Pages){%>
<tr class="<%=Html.GetRowClass(rowIndex) %>">
<td><%=p.Title%></td>
<td><%=p.Permalink%></td>
<td><%=p.Created.ToLongDateString()%></td>
<td><a href=#>Delete</a></td>
</tr>
<%rowIndex++; %>
<%} %>
This is not optimal, however - I don't like the extra variable in there. It feels hacky. Also, I'm missing the condition where the Page is inactive.
To get around this, I'm going to extend the Page class (since it's a partial to begin with) to have a property called "ListIndex". This may sound hacky as well - but there's a lot of reasons to have this property, so I'm OK with it.
Next, I'll add an overload for the GetRowClass() that takes the Page record itself:
public static string GetRowClass(this HtmlHelper helper, AtwoodPost.Models.Page page)
{
return page.IsActive ? GetRowClass(helper, page.ListIndex) : "inactive-row";
}
Now I can use this, and tidy up the UI code just a bit more:
<%foreach(AtwoodPost.Models.Page p in ViewData.Model.Pages){%>
<tr class="<%=Html.GetRowClass(p) %>">
<td><%=p.Title%></td>
<td><%=p.Permalink%></td>
<td><%=p.Created.ToLongDateString()%></td>
<td><a href=#>Delete</a></td>
</tr>
<%} %>
Ahh this looks much better. In the AdminController I've set up the stub data to have a proper ListIndex (something you can do by using a simple loop) and I've also made it so that every 6th record is inactive. Let's take a look at it now:
Pretty good if you ask me!
Final Step: Adding Delete/Confirm Logic
The final thing we need to do here is to add some logic to delete the entries. Right now it's just stubbed out in a blank anchor tag. Let's fix that!
For now, assume there is a Controller method called "Delete()" which takes a PageID (integer). The functionality we need to create here is that when a user clicks "Delete", a confirmation dialog pops up and asks the user to confirm. If they say yes, the Delete() action is called.
One thing that's hard to get used to is that ASP.NET MVC let's you use multiple forms on a page and that's what I want to do here. I can use a URL to do this if I want, but I don't like exposing delete functionality by URL - seems like a bad thing. I'm also going to use an image here - the repeated word "Delete" looks like noise. This is the changed code:
<%foreach(AtwoodPost.Models.Page p in ViewData.Model.Pages){%>
<tr class="<%=Html.GetRowClass(p) %>">
<td><%=p.Title%></td>
<td><%=p.Permalink%></td>
<td><%=p.Created.ToLongDateString()%></td>
<td>
<%using (Html.Form<AtwoodPost.Controllers.AdminController>(x => x.Delete(p.ID),
FormMethod.Post, new { onsubmit = "return confirmDelete()" }))
{ %>
<%=Html.SubmitImage("delSubmit","~/Content/delete.gif") %>
<%} %>
</td>
</tr>
<%} %>
I can feel Jeff squirming in his new Mirra, so I'll tweak this a bit to make it more readable by creating a new Extension Method specifically for a Delete form:
public static string DeleteForm(this HtmlHelper helper, string controller, string action, int id)
{
UrlHelper url = new UrlHelper(helper.ViewContext);
string postAction = url.Action(action, controller, new { id = id });
string formFormat = "<form action=\"{0}\" method=\"post\" onsubmit=\"return confirmDelete()\">";
return string.Format(formFormat, postAction);
}
Now you may be saying "ACK! There's HTML in there!" and that's true. I hate mingling HTML in C#. I would have just let it stay with the using above (the lamda and method call are clear to me), but in the interests of cleaning up the view I put this special form in its own method. As someone said to me once: "HTML is pretty static stuff - it's not going to change a lot" and I think putting some in code is OK.
Now The page code looks like this:
<h1>Posts</h1>
<table cellpadding="5" cellspacing="0">
<tr>
<td>Post</td>
<td>Permalink</td>
<td>Created</td>
<td>Option</td>
</tr>
<%foreach(AtwoodPost.Models.Page p in ViewData.Model.Pages){%>
<tr class="<%=Html.GetRowClass(p) %>">
<td><%=p.Title%></td>
<td><%=p.Permalink%></td>
<td><%=p.Created.ToLongDateString()%></td>
<td>
<%=Html.DeleteForm("Admin","Delete",p.ID)%>
<%=Html.SubmitImage("delSubmit","~/Content/delete.gif") %>
</form>
</td>
</tr>
<%} %>
<tr>
<td colspan=4><%=Html.NumericPager("/admin/index/{0}",ViewData.Model.TotalPages,
ViewData.Model.CurrentPage) %></td>
</tr>
</table>
The DeleteForm() method relies on a little convention here, and that is that there's a function on the page called "confirmDelete()". There is, and I put it where script should go, at the bottom of the page (and NOT in a method call, as the Typo example has it above):
<script type="text/javascript"> function confirmDelete(){ return confirm("You are about to delete this record. This is permananent. Are ABSOLUTELY sure?"); } </script>
And now we have a sexxy page:
Summary
I've always said: "Spaghetti is as Spaghetti Does" and I hold to that. I think I've pared this sample down pretty well, and made the page insanely clear; though I do recognize I can go a lot farther here. Jeff's concern about mingling HTML and Javascript it valid - and I don't think I've fully addressed his issue with what I've done here.
But we're not even at beta yet! Part of the exercise of the Storefront is to hear what you have to say - even if you have a wrecking ball over your shoulder :).
I wrote the above code over the course of 90 minutes, and I know that if I was to keep going I could roll out some sexxyness that Jeff would invite to share his chair with him. My ultimate point goes along the lines of the Uncle Ben Principle:
With great power comes great responsibility
You've been given the wheel to the Ferrari and some groovy leather gloves to grip the wheel with. It's up to you to sharpen your driving skills now so you don't fly off the cliff.
Frameworks don't create Tag Soup. Developers do.
