I've created the worst tag soup of all time (ASP.NET MVC 2)_问答_开发者

I am staring at the worst mess, my monitor isn't tall enough to see what is going on, and VS 2010 is no help at all.

I do not have any idea how to refactor this trash.

It's 11 AM and I feel like pouring a drink already.

Is this just the final proof that I have no business coding? Be honest.

<div id="followedFriends">
    <% if (Model.FollowedFriends.Count() > 0)
       {
           foreach (var friend in Model.FollowedFriends)
           { %>
    <div id="friendContainer">
        <div class="followedFriend">
            <div class="userName">
                <%= Html.ActionLink(friend.FoFriend.UserName, "Visitor", "Home", new {userID = friend.FoFriend.UserId}, null)%></div>
            Currently reading:
            <br />
            <div class="bookLinks">
                <% if (friend.BookCount != 0)
                   { %>
                <% if (friend.BookCount <= 5)
                   { %>
                <%= friend.BookLinks%>
                <%}
                   else
                   { %>
                <%:Html.ActionLink(friend.BookCount + " different books.", "Visitor", "Home", new {userID = friend.FoFriend.UserId}, null)%>
                <%}
                   }
                   else
                   { %>
                Nothing, it appears...
                <%}%>
            </div>
            <%if (friend.ReviewCount != 0)
              {%>
            New review for:
            <div class="reviewLinks">
                <%if (friend.ReviewCount <= 5)
                  { %>
                <%= friend.ReviewLinks%>
                <%}
                  else
                  {%>
                <%: friend.ReviewCount %>
                different books
                <%开发者_如何转开发}%></div>
            <%}

              if (friend.QuoteCount != 0)
              {%>
            <span class="highlight">&#9656</span>
            <%: friend.QuoteCount%>
            new
            <%if (friend.QuoteCount != 1)
              { %>quotes
            <%}
              else
              { %>
            quote
            <%} %>
            <%}%>
        </div>
    </div>
    <%}
       }%>
</div>
<%} %>

Update

Since someone was asking, here is the pertinent portion of the View Model:

 public class FollowedFriend
    {
        public aspnet_User FoFriend { get; set; }
        public string BookLinks { get; set; }
        public int BookCount { get; set; }
        public string ReviewLinks { get; set; }
        public int ReviewCount { get; set; }
        public int QuoteCount { get; set; }

        public FollowedFriend(Guid userID, DateTime lastVisit)
        {
            using (var context = new BookNotesEntities())
            {
                FoFriend = context.aspnet_Users.SingleOrDefault(u => u.UserId == userID);
                var reading = context.Books.Where(b => b.UserID == userID && b.CurrentlyReading == true).ToList();
                BookCount = reading.Count;
                if (BookCount <= 5)
                    BookLinks = Book.ConvertBooksToLinks("Book/Details", reading);
                else
                    BookLinks = "";
                var recentBooks = context.Books.Where(b => b.UserID == userID
                    && b.Review.DateCreated >= lastVisit).OrderByDescending(b => b.DateCreated).ToList();
                if (recentBooks.Count <= 5)
                    ReviewLinks = Book.ConvertBooksToLinks("/Book/Details", recentBooks);
                else
                    ReviewLinks = "";
                ReviewCount = recentBooks.Count;
                QuoteCount = context.Quotes.Count(q => q.UserID == userID && q.DateCreated >= lastVisit);
            }
        }
    }

Let's start with a maxim that was initially proposed by Rob Conery: if there's an embedded "if" somewhere in your View, it's probably an indicator that you should either A.) create an HtmlHelper or B.) retract the logic from the view and insert it into your ViewModel.

By doing so, you can clean up your View to look something like this (you get the idea):

   <div id="followedFriends">
        <% foreach (var friend in Model.FollowedFriends) { %>
        <div id="friendContainer">
            <div class="followedFriend">
                <div class="userName">
                    <%: Html.ActionLink(friend.FoFriend.UserName, "Visitor", "Home", new {userID = friend.FoFriend.UserId}, null)%>
                </div>
                Currently reading:

                <br />
                <div class="bookLinks">
                    <%: Html.DisplayBooklinks(friend) %>
                </div>
                <div class="bookReviews">
                    <%: Html.DisplayBookReviews(friend) %>
                </div>
                <div class="bookQuotes">
                    <%: Html.DisplayQuotes(friend) %>
                </div>
            </div>
        </div>
        <% } %>
    </div>

At this point, if there's any chance that this piece of UI might be consumed in some other page, you should consider throwing it into a user control. By doing so, your view could now look something like this:

<% Html.RenderPartial("FriendDetails", Model.FollowedFriends); %>

Ultimately, whenever your View starts to look like soup, it's because your view is doing too much thinking. Which, in turn, means: some other layer of your application isn't thinking enough. By pinpointing logic in your Views, and by determining what abstractions might aid you in your attempt to remain DRY, your Views will become much more readable and much more maintainable.

I would factor some of that inline logic to an HtmlHelperExtension method and then create a DisplayTemplate "Friend"

<div id="followedFriends">
    <% if (Model.FollowedFriends.Any()) {
        foreach (var friend in Model.FollowedFriends) {
            <%=Html.DisplayFor(friend) %>
       <% } %>
    <% } %>
</div>

And your DisplayTemplate "Friend" would look something like this:

<div id="friendContainer">
    <div class="followedFriend">
        <div class="userName">
            <%= Html.ActionLink(friend.FoFriend.UserName, "Visitor", "Home", new {userID = friend.FoFriend.UserId}, null)%></div>
            Currently reading:
            <br />
            <%=Html.BookInformation(friend) %>
            <%=Html.BookReview(friend) %>
            <%=Html.Quotes(friend) %>
        </div>
    </div>
</div>

Rather than making HtmlHelperExtensions you could just create more DisplayTemplates and specifically call them: DisplayFor("BookReview", friend)

Let me start by saying, we've all been there...

For your particular problem, I'd consider a few of the following "tools":

Partial Views: when you have a lot of nested <div> tags, for instance, you may want to consider creating some partial views where appropriate.
Use the Controller for Business Logic: Some of your logic should be done in the controller, where it's easier to break things up and call different helper methods to apply the logic. Consider passing to your view a custom object (a ViewModel if you will) that has all the business logic parsed. For example, things like displaying "quote" vs "quotes" could be done in a ViewModel object's "DisplayValue"-type property.
HTML Helpers: These little extensions can help take care of extracting the many if/else statements that you have into an easily readable method - and they could be re-used on other pages if written generally enough.

Lastly, I'd say, go get some fresh air and come back with only one thing in mind: simplify! Often we get caught up with a long list of tasks that need to be done and we get lost in our own soup or spaghetti or what-have-you. You'll need to approach the code with only refactoring in mind, until it's clean enough that you can tell where the next piece of logic should go.

I'm new to ASP.Net MVC, so I try to find coding conventions for me, too:

write <% %> tags in a separate line (except for <%: and <%=)
eliminate sequences of %><% (except for <%: and <%=)
keep C# and HTML indentation separate
indent <%= and <%: as if they where HTML tags (if they do represent HTML tag/s)

I'm not a ASP expert, but like in PHP isn't there a way to echo things? In place of using so much open and close tags? I should place everything above the code, and make variables that you use in your HTML?

In my opinion, this isn't awful. I think the key here would be to try to spend some time walking through it and commenting where blocks start and then their corresponding end tags. It'll be painful now, but it'll save you some time later on.

I'm assuming you're looking for a code review. In that case: