Hanalei, Hawaii 9/2/2010
438 Posts and Counting

ASP.NET MVC: Avoiding Tag Soup

Sunday, July 27, 2008 -

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:

typocode

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:

 Project

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:

grid1

 

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:

grid2

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:

 grid4

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.

Related


Gravatar
Ben Scheirman - Sunday, July 27, 2008 -

Amen! I hate that we've grown a culture of web developers who don't like HTML or Javascript. That's scary.

I do agree that <% %> syntax can be very heavy on the view, and used too liberally can really hurt readability. This is why I find NVelocity to be so refreshing.

Consider this (in WebFormsViewEngine):

<% if(ViewData["Username"] != null) { %>

<div class="login-details">

Logged in as <%= ViewData["Username"] %>.

<%= Html.ActionLink<LoginController>(c => c.Logout(), "Log out") %>.

</div>

<% } %>

with this (NVelocityViewEngine) :

#if ($username)

<div class="login-details">

Logged in as $username. $html.actionlink("Log out", "login", "logout")

</div>

#end

Granted, I like the readability of the lambda for the action link, but otherwise the 2nd example far exceeds the first in readability. We're also sacrificing compilation and intellisense here, but I'll go out on a limb and say they aren't needed in a lot of scenarios.

Gravatar
Padgett Rowell - Sunday, July 27, 2008 -

Nice refactoring. The only bit that makes me a little queasy is the errant </form> tag here:

<%=Html.DeleteForm("Admin","Delete",p.ID)%>

<%=Html.SubmitImage("delSubmit","~/Content/delete.gif") %>

</form>

It would be nicer if it looked like this:

<%=using (Html.DeleteForm("Admin","Delete",p.ID)) {%>

<%=Html.SubmitImage("delSubmit","~/Content/delete.gif") %>

<%} %>

Gravatar
Mohammad Azam - Sunday, July 27, 2008 -

Hi,

Actually, I agree with Jeff's post. Sure we have extracted out all the code from the page behind and now we can test our application. But the cost is too high. Now, we have to do everything on our own. Our code behind is clean but guess what our view is freaking ugly. What we don't realize is that the real power of the ASP.NET framework are the controls. You remove the ASP.NET controls and you are left with nothing.

Rob, you wrote so much code just to achieve paging. How about you implement the GridView in-place editing now. Let's see how much code you end up writing?

I am not saying that ASP.NET MVC is BS. I am just saying it won't work when your view is really complicated.

One of the other things that we need to realize is that the MVC Rails framework (default rails framework) is not so popular because of the the MVC implementation. It is popular because of the Ruby language. Ruby is a very concise language which makes things happen quickly.

Using ASP.NET MVC seems like I have transported myself 10 years back to the days of classic ASP.

Gravatar
Chris Hardy - Sunday, July 27, 2008 -

Just something to mention which you might want to change - the overloaded GetRowClass method has inside it the first GetRowClass however it's named GetRowFormat, could be confusing if people are following it through.

@Mohammad - You can create your own helpers just as ASP Controls would work - except that they'd be a lot more flexible. Sure, it's a lot more code to start off with but then you're free to change it how you wish - and no doubt you'll then re-use it on another project and it'll be time well spent.

Gravatar
Rob Conery - Sunday, July 27, 2008 -

Hi Mohammad - thanks for the comments:

>>>But the cost is too high. Now, we have to do everything on our own<<<

You don't have to - MVC is for the folks who like that sort of thing.

>>>What we don't realize is that the real power of the ASP.NET framework are the controls. You remove the ASP.NET controls and you are left with nothing.<<<

It depends on how you define "power". I don't define it with controls - I define it by extensibility and testability.

>>>Rob, you wrote so much code just to achieve paging. How about you implement the GridView in-place editing now. Let's see how much code you end up writing?<<<

Like I said in my post - if I go down the WebForms rabbit hole and pop DataSets and/or SqlDataSources on my page - yah, I can do a lot. Can't server-side page though, and to do that I'd write an order of magnitude more code.

>>>I am not saying that ASP.NET MVC is BS. I am just saying it won't work when your view is really complicated. <<<

What do you find complicated?

>>>Using ASP.NET MVC seems like I have transported myself 10 years back to the days of classic ASP.<<<

Ack. There it is again :). With all due respect, you can keep on truckin with WebForms and the GridView (you are the GridViewGuy yes?).

Better yet - you've read the post, you know the requirements :). Let's see a GridView :) response to this...

Thanks again for the thoughts.

Gravatar
Rob Conery - Sunday, July 27, 2008 -

@Chris - thanks! Fixed!

@Padgett - agree with you on the </form> stuff.

Gravatar
Craig - Monday, July 28, 2008 -

>>>One of the other things that we need to realize is that the MVC Rails framework (default rails framework) is not so popular because of the the MVC implementation. It is popular because of the Ruby language. Ruby is a very concise language which makes things happen quickly.

I have to disagree with this. Nobody used and hardly anyone had heard of Ruby until Rails came along. Rails is precisely the reason RoR is so popular.

Gravatar
Paul Linton - Monday, July 28, 2008 -

Rob,

Right on the money, as usual. I've always felt uncomfortable about any tag soup that I create. My personal test is to imagine walking a colleague with basic skills through the code and thinking about ways to make the walkthrough easier.

I have just modified my own paged lists to use the style you have presented, much nicer now and easier to explain to someone else. The piece I was still a little uncomfortable with was extending the model with the ListIndex property which is view related. So, I have modified PagedList to inherit from List<PagedListItem<T>> instead of from List<T>. PagedListItem<T> looks like

public class PagedListItem<T>

{

public T Item {get; private set;}

public int ItemIndex {get; private set;}

public bool Odd { get; private set; }

public string CssClass { get; private set; }

public PagedListItem(T item, int index)

{

Item = item;

ItemIndex = index;

Odd = index % 2 == 1;

CssClass = Odd ? "odd" : "even";

}

}

I have modifed the PagedList ctor from

.... .Take(pageSize).ToList();

to

.... .Take(pageSize).Select((s,i)=>new PagedListItem<T>(s,i+1)).ToList();

In the view I now use

<% foreach(PagedListItem<Company> company in ViewData.Model.Companies) { %>

<li class="CompanyItem <%=company.CssClass%>"><%=company.Item.Name%></li>

<% } %>

Thanks for the clarity of this post.

Gravatar
Jamie - Monday, July 28, 2008 -

Rob said: "where script should go, at the bottom of the page"

I always thought it goes in the head section? I got in that habit quite awhile ago to be sure the script was ready when needed. Is that no longer the way the cool kids do it?

Gravatar
Rob Conery - Monday, July 28, 2008 -

LOL whoops! What I meant to say was "not in the code or mingled in HTML". I didn't mean to single out the bottom of the page - nice catch :)

Gravatar
Dan Finch - Monday, July 28, 2008 -

I agree - a little discipline will keep the spaghetti under control. And MVC can't be evaluated solely on these grounds, you can use an alternative ViewEngine or still use custom server controls, TagPrefixes and all (though it's a little more hands-on, thankfully).

Gravatar
Robert - Monday, July 28, 2008 -

Rob,

I am actually digging MVC.NET, precisely because of its ClassicASP-like simplicity. Closer to the metal. Anyway, Ben Scheirman is onto something with Velocity. I'd like to propose taking it even further: writing zero code in the view, just HTML. Have absolutely all the code in the model and the controller. The code would then treat the View as a simple template to find and replace tokens.

The pattern (as used in classic ASP) is in the link below. I used it quite extensively during that era and it worked out absolutely great: the code was testable (as testable as VB6 code could be), very maintainable and easy to grok.

How to create ASP applications without writing ASP code www.vbrad.com/.../pf.asp

Gravatar
ABN - Monday, July 28, 2008 -

Rob,

Are you planning to provide download link for this sample?

Thanks.

Gravatar
Torkel - Monday, July 28, 2008 -

Good post Rob, I agree! But I think the WebForms view engine is making it extra difficult in making views readable.

Just a couple of days ago I switched from the WebForms view engine to the Spark view engine, the views got a lot smaller and are much more readable.

I love this:

<tr each="var page in pages">

<td>${page.Title}</td>

</tr>

For a comparison between one WebForms view that I converted into a Spark view: www.codinginstinct.com/.../mvc-viewengines

The nice thing about Spark is that some of the steps you go through in this post will not be necessary :)

Gravatar
Rob Conery - Monday, July 28, 2008 -

Thanks Torkel - Jeff mentioned Spark in our conversation and I took a look at it. To be honest it seems a little weird to see code mixed with HTML like that - especially when this is what Jeff's rebelling agianst. But I do get it - it's nice.

Gravatar
Mihai - Monday, July 28, 2008 -

Hey.

My opinion is that MVC is a powerful framework, and indeed if someone wants to write bad code, or code that doesn't fit in someones philosophy of doing something he/she/it will.

One of my professors argued about not wanting to write "php code" in one of his lectures because he once saw some kid in junior high write 17 if( .. ) else .. to do some kind of logic. The way he said it sounded allot like a "don't write php" so obviously I argued his argument since at the time I was using Joomla! who has a great MVC framework ( similar to this one :P )

I've read through Jeff Atwood's blog and I don't think it's a bad thing he's showing how "if things can go wrong, they will".

You've shown a good practice in your blog, but with the idea of getting things done more examples similar to what Jeff showed are bound to appear simply because they can :D.

Also I don't think a mechanism for not allowing something like that would do any good if implemented inside MVC.

The only way we could avoid it is if we would use a completely different view engine like NHaml or something else that generates the final html.

The same thing happens with JSP, JSF, PHP, Ruby etc..

So long as bad practices come along with a good practice "coding horrors" like the one we've just seen on Jeff's blog will ("we can only hope") be kept in check.

Gravatar
Mike - Monday, July 28, 2008 -

sb.AppendFormat("<b>{0}</b>", pageLink);

...fail

Gravatar
Mike - Monday, July 28, 2008 -

There are so many sides to this discussion, and much of it has been discussed many times over. Here's several ways of looking at it. It seems there cannot be one conclusion, only repeating of arguments (yay for pageviews!)

First of all there exists a misunderstanding about mixing code and markup. Some people think it's bad, but that's based on actual bad code they have written or seen (like opening and closing record sets in a classic ASP page). If you can limit the code to presentational logic, it is 'ok' but even then the code can get messy. (It reminds of the discussion on Hungarian notation, system vs. apps, remember that?)

Some don't like using the gridview control because you have to work with all the event, but in fact you can use helper methods with a gridview too.Some argue that the gridview is too verbose, but with intellisense you can code one in no time.

Some say they don't like HTML to be abstracted, but then what is the purpose of the HTML helper methods? Convenience sure, but it can look like abstraction as well. Take a look at the Ruby on Rails example, it does :confirm => "Some string" to 'abstract' the javascript.

Code behind is frowned upon by some MVC developers (see this post), but with a lot of care you can actually have quit a clean 'page controller' and use some inline code in the .aspx file to do some logic. Take enough care and it might even look good (MVC like). Take the MVC route and you are advised to write helper method where you String.Format your way through snippets of HTML. Don't be surprised if people criticise that, because it has bit of code smell.

Today we see there are many view engines. What is the implication of that? They offer a lot of convenience, but you can't step through them with the debugger. Are they even compiled? It could be that under load you will find that the view engine spends a lot of time interpreting the 'smart' notation they came up with.

Talking of view engines, this is some spark I saw the other day, I'll let the code speak for itself:

<test if="expression">

...

<else/>

...

</test>

Some people really like webcontrols, copy a dll and you have charts or ajax running. Though it's sad to see the lack of quality open source control libraries, with some money you can get some great stuff. MVC has yet to show any potential in that area.

Gravatar
Slava - Monday, July 28, 2008 -

thanks for the post.

my main concern with this is that it isn't apparently clear what best practices are with MVC.

the way Jeff did things is the way newcomers to MVC will do things. they won't know right away about using extensions and helpers to create better code. eventually, if they stick with MVC or read blog posts like these or just spend extra time thinking they'll figure out how they can improve their view code. or they'll get scared away by how they can make things go wrong.

in webforms things can go wrong too, but that isn't apparent until much, much later:) making webforms more likable at first.

it'd be cool if there would be a single place (wiki, some official book, whatever) for developers (especially if they're new to MVC) to look at to see all the practices and techniques to make their code better. it would help with a lot of problems people are facing.

Gravatar
Steve C. - Monday, July 28, 2008 -

@Jamie, @Rob - Rob you were correct in saying that the script should be at the bottom of the page. Scripts block parallel downloads, and as such they should be added to the bottom of the page. This allows for the page to load quicker, saving the executing of any scripts until the page is completely loaded. Check YSlow's docs.

As for the other comments re: Tag soup. I've said this from the start about MVC - MVC is great, but so are ASP.NET controls. I don't want to have to manually construct the HTML myself, and no I don't fear HTML. I would consider myself to be an expert at HTML/XHTML and JavaScript. I just want to work with a framework where I don't have to spend time on the little stuff. I'm not saying we should have something like a GridView. They are complete overkill, and the markup sucks unless you use the CSS control adapters, which just adds more code to your app.

I just want a great, testable framework that minimizes the amount of code I write - all the code. This includes the HTML too.

Gravatar
Brian Henderson - Monday, July 28, 2008 -

Thanks Rob. Lots of great comments make this a nice refreshing post to read on Monday morning.

A take away is: a clear separation of concerns between 'layout' and data 'logic'. Look forward to seeing more enlightening post... for example; using a 'view template' for editing a row in the (above) grid? :)

Gravatar
Alan Stevens - Monday, July 28, 2008 -

Rob, Excellent post! We need more examples like this to get the story of MVC across to WebForms junkies. You have done a great job of demonstrating why having more control leads to more maintainable code.

++Alan

Gravatar
Paco - Monday, July 28, 2008 -

Good post Rob!

I was wondering why you use Webhelper.Numeric pager instead of a ViewUserControl.

With these htmlhelper methods you move your UI logic away from the views itself. I understand this is done for really small things like Html.ActionLink and other methods that created only one html tag.

In my opinion you can use a htmlhelper extension method if it writes just one simple string, but if you need a stringbuilder, you should consider creating a ViewUserControl.

When is this a violation of MVC?

Gravatar
Rob Conery - Monday, July 28, 2008 -

Nice point Paco - as opposed to Mike above, I don't mind putting some simple markup in code - there's always a violation somewhere :). But I do agree - if you have buttons and dropdowns etc, you should put this into a user control for sure.

Gravatar
Tim C - Monday, July 28, 2008 -

Wait. What?

"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)."

What about alternating row style?

<asp:GridView RowStyle-BackColor="AliceBlue" AlternatingRowStyle-BackColor="Wheat" AutoGenerateColumns="false" ID="GridView1" runat="server" AllowPaging=true onpageindexchanging="GridView1_PageIndexChanging">

<Columns>

<asp:TemplateField HeaderText="Title">

<ItemTemplate>

<%# TrimTitle("Title") %>

</ItemTemplate>

</asp:TemplateField>

<asp:BoundField DataField="Date" HeaderText="Date" />

<asp:BoundField DataField="Link" HeaderText="Link" />

</Columns>

</asp:GridView>

Notice that I also am DataBinding to a function called TrimTitle which is in my CodeBehind. I Could do this inline too...

I didn't have to write any rowdatabound event stuff in my codebehind. Am I missing something here?

Gravatar
Rob Conery - Monday, July 28, 2008 -

Thanks Tim for pointing that out - a lot of people would assign this to an OnRowDataBound spelunking event and you are correct - you can easily do this with a # lookup. However the last part of what I said:

>>>that would most likely be buried in the CodeBehind and not centralized and reusable<<<

is, ironically, exactly what you did :). But your point is taken.

I should also add that I know you CAN do this in webforms- the challenge is to do it readably and maintainably. In looking at the code here, I still prefer the scripting I've done. To each his own...

Gravatar
Vasquez - Monday, July 28, 2008 -

How would you approach variable numbers of columns using the MVC approach?

Gravatar
Eric - Monday, July 28, 2008 -

What about performance and overhead?

Does it take longer to parse out the inline code and generate the compiled page? Or does it take longer to parse out the control definitions, generate the controls, and call render on each control?

Has anyone started to look into the performance and scalability aspects of moving from the ASP.NET Web Forms model (heavily control based) to this new inline model?

Just curious...

Gravatar
OmegaSupreme - Monday, July 28, 2008 -

Why don't we just have something called "Controls" and whack all our html and code into there. Heck you could also invent some kind of page life cycle and event model to prevent us having to create endless controller actions. Also I can't be the only one finding the lack of state between requests frustrating... BTW I'm being sarcastic but I do predict the old model coming back into fashion .. though I hope not.

Gravatar
Rob Conery - Monday, July 28, 2008 -

Hi Vasquez - Can you give me a scenario? I'll do my best to come up with something...

@OmegaSupreme: you got me! There are meetings where the conversation drifts that way (along the lines of "we need some kind of Repeater"). The good thing is that WebForms does what it does - MVC does it a bit differently. And it's not even beta yet!

Gravatar
Ai_boy - Monday, July 28, 2008 -

Good post! Wait more ^_^

Gravatar
Lance Fisher - Monday, July 28, 2008 -

Rob, I love your similes! "like cats with a spray bottle", "dresses on their kittens". I agree that MVC code can be much more straight-forward than webforms, and I like it a lot.

Ultimately, we are writing code (C#) to write code (HTML) so there's bound to be a mixture somewhere, but it's good to keep as separate as possible. I like your "If there's an If..." policy - I'll have to adopt that. I like to see HTML in my view, and I opt for that whenever possible. I don't really like using the "using" to create forms. I use the HTML element, and the Url.ActionLink() instead, but I am liking the options. I would like it if a designer (with HTML experience of course) could read the view.

As for the JavaScript, I like that ASP.NET MVC doesn't mess with my HTML IDs so I can make better use of js. It's wonderful to use jQuery and YUI with ASP.NET MVC. I try to put my js in an external file. This is usually the best approach, but there are exceptions. I just don't want to ever see an "onclick='...'". In your example, I would get rid of the "onsubmit=" and hook up the event handler with js.

Gravatar
k_man - Tuesday, July 29, 2008 -

Great example.

But I am looking at it another way. Why in 2008 do we still have to implement paging, alternating styles and delete confirmations at all? This should all be built in and enabled by default.

And it should be styled to look good. Think of

Matt Berseth's Google grid and other cool stuff, like his VS2008 grid. That is what we should have by default. Why do Flash developers have nice looking stuff be default and we have crap.

Gravatar
Rob Conery - Tuesday, July 29, 2008 -

>>>Why in 2008 do we still have to implement paging, alternating styles and delete confirmations at all<<<

It's our job?

>>>This should all be built in and enabled by default. <<<

With the GridView, a lot of this is. Out of curiosity - when you say "built in" - are you meaning built into HTML? The Framework? It can be - and I have no doubt someone will come up with it. MVC's not in beta yet ...

>>>Why do Flash developers have nice looking stuff be default and we have crap.<<<

Are you asking why Flash looks nicer than HTML?

Gravatar
GrantM - Tuesday, July 29, 2008 -

>>> >>>that would most likely be buried in the CodeBehind and not centralized and reusable<<< <<<

>>>is, ironically, exactly what you did :). But your point is taken.<<<

I'm just seeing logic being buried in a controller instead of a code behind.

If something was to made reusable, there's no reason it can't be placed in it's own class, or used as an extension method.

Gravatar
Me - Tuesday, July 29, 2008 -

I'm not very experienced with ASP.NET, so all this controls vs inline stuff is raising a question:

If you wanted to create a custom control, wouldn't you have to write exactly what seems to be frowned upon by both sides: C# code that outputs HTML with String.Format -style stuff? What if you wanted to change the way a control renders itself?

Tried looking on Google, but for some reason I wasn't able to find anything that really showed the stuff.

Gravatar
Richard - Tuesday, July 29, 2008 -

Nice post, nice discussion. What I don't like too much in the sample are the magic strings like "/admin/index/{0}" you pass into the NumericPager. This way the view knows quite something about where it comes from. Can this be done differently, using lambda's or so?

Gravatar
Brennan Stehling - Tuesday, July 29, 2008 -

I think the complaints are off the mark. Mixing in all of this code in with HTML is bother people and it seems they cannot properly communicate why it bothers them. They know it is hard to maintain and claim it is due to lack of readability, but it is more than just that. When you look at this code you wonder about Intellisense and debugging support. The benefit of putting any logic into the Code-Behind or in a class library is that you get the benefits for Intellisense and the ability to set break points for debugging. That is what is implicitly missing from the markup/code mix shown here.

Currently when I open a code declaration in ASP.NET I do not get Intellisense support. And a minor pet peeve of mine is that in order to attach event with IDE support I have to go to design mode and select the event from the Properties window. I cannot do that from the text edit mode which should be there since it can clearly identify the control tag that is highlighted. That aside current shortcoming could be extending to any place where I see HtmlHelper which leads developers to feel they are losing the power of the IDE. They may as well be using Notepad to edit their code.

Being able to put in any code in any way you like does give the developer a lot of power. But without giving them assistance to assemble all that code you also require them to remember quite a bit about complex HTML semantics. Code snippets could fill this void and an improved syntax could improve how the code looks and enhance IDE integration. When I see string hard-coded in a page red flags go up for me. If these control variables are configured in the Web.config I would like some hint at whether these values are valid or possibly not.

I have many more thoughts on this but I have to dig into the latest preview before I can put those thoughts together so they are useful to others. Fortunately Jeffrey Palermo is coming to speak to our local .NET User Group on MVC which should help me get a jump start on things again. I hope a lot changes before the first beta of MVC. I see areas where it could improve dramatically.

Gravatar
Vasquez - Tuesday, July 29, 2008 -

Hi Rob - re:variable columns question

Basically I was trying to figure out how you could hide/show columns dynamically like you can with a GridView. I'm also wondering how to use different templates for each column. The templates would also have to be changeable depending on the value in the cell, e.g. apply a CSS class depending on a value.

Gravatar
Rob Conery - Tuesday, July 29, 2008 -

Hi Brennan - with VS 2008 you get full intellisense and debugger support in each page - in fact intellisense discovery is what drove a lot of the helper API.

@Vasquez - I've never seen dynamic column hide/show, but I'm sure it's possible (easily) with Javascript.

Gravatar
Vasquez - Tuesday, July 29, 2008 -

Hi Rob - re:variable columns question

I know it's possible to do this with javascript, but I want to do it server side. They are columns that only appear when a user is logged in.

And I still need to figure out how to change the html for given cell contents. Imagine an online bank statement that showed black or red text depending on the balance.

This is the sort of the thing that is trivial with a datagrid/gridview using column templates but seems like it would involve lots of html embedded into the c# code using ASP.Net MVC and that just seems wrong to me.

Gravatar
Rob Conery - Tuesday, July 29, 2008 -

Ah I see what you mean. Yes, in your case you'd need to wrap the columns with an "if", which may not look ideal and I, for one, HATE seeing "if"'s in views. But all the same, it's two extra lines of code, and the cost is minimal.

In terms of it "just seeming wrong" - a lot of people have this reaction and it's understandable. What a lot of people forget, however, is the monstrous amount of code that's in the GridView. I don't mean that in a bad way, but consider how much functionality is being thrown at a rather simple problem.

Anyway - it boils down to what you're comfortable with, and if you don't like tagging the View it really is OK. I think if all I do is get people to try it once and experience something new - well then all in the world is good.

Gravatar
Dragan Panjkov - Wednesday, July 30, 2008 -

Stuck on step two... creating paged list from list. I had one big list displayed, but I ran into problems when I tried to implement pagedlist approach... code download would be highly appreciated :)

Gravatar
Rob Burke - Wednesday, July 30, 2008 -

Rob,

Any chance of you zipping up the code for this and releasing it? It's the kind of really basic intro to MVC I'd love to show my IT Manager.

Cheers,

Rob

Gravatar
Dragan Panjkov - Wednesday, July 30, 2008 -

I just created zip archive for this sample. More info: weblogs.asp.net/.../avoiding-tag-so

Gravatar
Dietrich - Wednesday, July 30, 2008 -

>>Some people really like webcontrols, copy a dll and you have charts or ajax running... MVC has yet to show any potential in that area.

With Rails the solution is "plugins". Couldn't be easier really and you can dig into the code to customize. Doesn't Asp.net MVC have an equivalent?

Gravatar
Shiju Varghese - Wednesday, July 30, 2008 -

Hi Rob,

Thanks for the post. Nice answer to amateur developers.

Gravatar
foobar - Thursday, July 31, 2008 -

Jeez, I can't read this blog. How big screens do you folks have? 30"?

Gravatar
Simon - Thursday, July 31, 2008 -

Just use NHaml

Gravatar
Rob Conery - Thursday, July 31, 2008 -

@foobar - some of the pictures are a bit large. You still stuck with a 15"?

@Simon - NHaml doesn't mitigate a thing - you still need to know when/how to "logic up" a view.

Gravatar
Rodrigo - Friday, August 01, 2008 -

Hi Rob I think the MVC framework would be a great aproach to use when migrating an application from asp to asp.net. Some parts of the old code could be used.

There are still lots of classic asp applications out there...

Gravatar
Kirk - Monday, August 04, 2008 -

Sitting here gnashing my teeth over a few comments, and can't bite my tongue any more:

For the folks bemoaning the lack of third party controls, come on. This product isn't even in Beta yet! What; do you expect third party vendors to start pumping thousands of dollars of developer time that might get completely wasted by the next preview when the ASP.NET team decides to change something in the framework? Not bloody likely, which is WHY you don't see any libraries yet. Patience is a virtue, grasshopper.

And for the folks wondering why MS didn't make the default web forms controls the uber-controls with all the secret sauce... That's not their job.

As the framework provider, Microsoft's job is to provide the basics. Anything beyond that is the purview of we the developer community or third-party vendors. If Microsoft created be-all-end-all controls, not only would they be accused of code-bloat, unnecesary features, 'stomping on the little guy', and possibly being dragged into court over anti-trust violations, but they wouldn't be providing for a robust developer community.

Gravatar
I Spose I’ll Just Say It: You Should Learn MVC - Rob Conery - CodeBetter.Com - Stuff you need to Code Better! - Wednesday, April 22, 2009 - [...] Is Full of Tag Soup? Once again, I’ll say this: Spaghetti is as Spaghetti does. I don’t like to see a bunch of [...]
Gravatar
Monkey - Monday, June 01, 2009 - As a seasoned web developer, I find this departure from webforms a huge step back. Esp since i use WCF as a service interface and unit test the boundary here. My UI is simplified. It's so much more complicated, I'm failing to see the advantages from a 'delivery based focus' And since when, did MS put unit testing first anyhow...
Gravatar
RPGKaiser - Thursday, August 13, 2009 - Hi guys, very interesting debate! I'm not a detractor of the spaghetti code but i think that they are a slowing point in the development cycle, and can limits the IDE benefices to the developers. I like MVC very much, due to the clearance you can reach in your application using it, but i won't be complete happy until i could use the helper methods much more transparent and IDE integrated (ex. Toolbox, Property Inspector, etc).
So, one idea came to my mind (although maybe some else already): what do you think about create something that make possible to treat the HTML helper methods like regular tags (ex. ), making and automated mapping between attributes and method parameters and appending the non mappable attributes to the root tag of the HTML obtained from the helper method call. Of course this (and only this) is not much different to the current use of HTML helper method, and in fact will eliminate the benefits of the intellisense. But, like the render engine have to be redesigned in order to support this approach, we could think into expand the intellisense scope to this kind of tags and attributes consequentially, and to populate a ToolBoxItem automatically with all the existent helpers, in order to enable the drag&drop functionality, like with any other regular ASP.NET control or component.
Well, like i said earlier, this could be someone else idea that i don't know about, so please, if is the case, give me some hint about it; and if isn't, help me to refine it, and (why not) to committed if not to much complicated.
Thanks very much for your attention!!
Gravatar
hvac - Monday, August 31, 2009 - nice sample, can we download the sample ?
Gravatar
PikesvillePaesano - Thursday, September 24, 2009 - awesome as always, but I'd can the GetRowClass:
public static string GetRowClass(this HtmlHelper helper, int rowIndex)
{
return rowIndex % 2 == 0 ? "normal-row": "alternate-row";
}

in favor of one line of jquery:
$("table re:nth-child(even)").addClass("alternate-row"):
Gravatar
Dale - Monday, November 02, 2009 - I'm new to MVC so I'm not sure if I'm doing something stupid or the MVC codebase has evolved since Rob wrote this, but I had to use:

UrlHelper url = new UrlHelper(helper.ViewContext.RequestContext, helper.RouteCollection);

to get the DeleteForm htmlhelper pattern Rob described to work.