Is there a prettier or more performant way to create an MVC Pager Helper?

Question

I've got this MVC Pager Helper and it's working exactly as expected. I'm wondering if there's a prettier or more performant way to build this out?

Also, I know that the StringBuilder might not be the best way, would a TagBuilder be a better approach?

    <Extension()>
    Public Function Pager(helper As HtmlHelper,
                          urlPrefix As String,
                          totalRecords As Integer,
                          currentPage As Integer) As MvcHtmlString

        ' Get out if we have 5 or less records
        If totalRecords <= 5 Then Return Nothing

        ' Make sure we're not getting invalid pages
        If currentPage <= 0 Then currentPage = 1

        ' Setup our initial variables
        Dim sb1 As New StringBuilder(),
            totalPages = (Math.Round((totalRecords / 5) + 0.5)),
            startingPoint,
            linksAfterCurrent,
            endPoint,
            i


        ' Set boundries for inner link numbers
        Select Case currentPage
            Case totalPages : startingPoint = currentPage - 5
            Case (totalPages - 1) : startingPoint = currentPage - 4
            Case Else : startingPoint = currentPage - 3
        End Select

        Select Case currentPage
            Case 1 : linksAfterCurrent = currentPage + 4
            Case 2 : linksAfterCurrent = currentPage + 3
            Case Else : linksAfterCurrent = currentPage + 2
        End Select

        sb1.Append("<div id=""pagercontainer""><ul class=""pager"">")

        ' Display the previous button and first button
        If currentPage > 1 AndAlso startingPoint >= 1 Then
            sb1.AppendLine([String].Format("<li><a href=""{0}1"" title=""go to page 1"">1</a></li>", urlPrefix))
            sb1.AppendLine([String].Format("<li><a href=""{0}{1}"" title=""go to page {1}"">&laquo;</a></li>", urlPrefix, startingPoint))
            sb1.AppendLine("&nbsp;&nbsp;&nbsp;")
        End If


        ' Generate the inner numbers
        i = startingPoint
        While (i < linksAfterCurrent)

            If (i >= 0) AndAlso
                (i < totalPages) Then

                sb1.AppendLine([String].Format("<li><a href=""{0}{1}"" {2} title=""go to page {1}"">{1}</a></li>",
                                               urlPrefix,
                                               i + 1,
                                               If(i + 1 = currentPage, "class=""youarehere""", String.Empty)))
            End If

            i += 1
        End While

        endPoint = i

        ' Display the next button and the last button
        If (currentPage < endPoint) AndAlso
            (endPoint < totalPages) Then
            sb1.AppendLine("&nbsp;&nbsp;&nbsp;")
            sb1.AppendLine([String].Format("<li><a href=""{0}{1}"" title=""go to page {1}"">&raquo;</a></li>", urlPrefix, endPoint + 1))
            sb1.AppendLine([String].Format("<li><a href=""{0}{1}"" title=""go to page {1}"">{1}</a></li>", urlPrefix, totalPages.ToString()))
        End If


        sb1.Append("</ul></div><div class=""clear""></div>")


        Return MvcHtmlString.Create(sb1.ToString())
    End Function


        sb1.Append("</ul></div><div class=""clear""></div>")


        Return MvcHtmlString.Create(sb1.ToString())
    End Function

Answer 1

How about something like this?

// Put this in its own class so that you can have private methods without being confused with your other HtmlHelpers
public class PagerGenerator {
    //extension method hook so this gets added to HtmlHelper
    public static MvcHtmlString Pager(this HtmlHelper helper, string urlPrefix,int totalRecords,int currentPage) {
        return new PagerGenerator(helper, urlPrefix).Generate(totalRecords,currentPage);
    }

    public MvcHtmlString Generate(this HtmlHelper helper, string urlPrefix,int totalRecords,int currentPage)
        if(totalRecords <= recordsPerPage)
            return null;
        if(currentPage <= 0) currentPage = 1;

        var totalPages = Math.Floor(totalRecords / recordsPerPage); //Math.Floor is the function you're looking for

        var startingPoint = CalcualateStartingPoint(currentPage, totalPages);
        var linksAfterCurrent = CalculateNumberOfLinksAfterCurrent(currentPage, totalPages);

        //I have no problem with using StringBuilder here actually, tagbuilder is fine too if you want, either should be fine.
        var sb = new StringBuilder(); 

        sb.Append("<div id=""pagercontainer""><ul class=""pager"">")
        AppendStartingLinks(sb, currentPage, startingPoint);

        // Personally I hate for loops. foreach is better. An alternative is to generate all i's, 
        // Linq-Select the strings you want, and to append them all in one shot, but this is probably a bit
        // easier to read
        foreach(var i in Enumerable.Range(startingPoint, linksAfterCurrent-1))
            AppendPageLink(sb, i, i.ToString(), i == currentPage-1? "class=""youarehere""" : String.Empty);


        AppendTerminalLinks(sb, linksAfterCurrent, totalPages);
        sb.Append("</ul></div><div class=""clear""></div>");


        return MvcHtmlString.Create(sb1.ToString())
    }


    // move this to a constant so it's not just a magic number floating around
    const int recordsPerPage = 5;

    private readonly HtmlHelper htmlHelper;
    private readonly string urlPrefix; //generally a  good idea to move to the constructor params that you're passing around a bunch
    public PagerGenerator(HtmlHelper helper, string urlPrefix) {
        this.htmlHelper = helper;
        this.urlPrefix = urlPrefix;
    }

    private static int CalcualateStartingPoint(currentPage, totalPages) {
        // Nothing wrong with case statements here, I just prefer ternary notation
        var startingPoint = 
            totalPages == currentPage ?     currentPage - 5 :
            (totalPages -1) ==currentPage ? currentPage - 4 :
                                            currentPage - 3;
        return startingPoint > 0 ? startingPoint : 0;

    }
    private static int CalculateNumberOfLinksAfterCurrent(currentPage, totalPages) {
        var l =
            1 == currentPage ?  currentPage + 4 :
            2 == currentPage ?  currentPage + 3 :
                                currentPage + 2        
        return l > totalPages ? totalPages : l;
    }
    private void AppendPageLink(StringBuilder sb, int page) {
        AppendPageLink(sb, page, page.toString());
    }
    private void AppendPageLink(StringBuilder sb, int page, string text, string additionalAttributes = null) {
        sb.AppendLine(String.Format("<li><a href=""{0}{1}"" title=""go to page {1}"" {3} >{2}</a></li>", 
            this.urlPrefix, page, text, additionalAttributes == null ? String.Empty : additionalAttributes));
    }

    private void AppendStartingLinks(StringBuilder sb, int currentPage, int startingPoint) {
        if(currentPage > 1 && startingPoint >= 1) {
            AppendPageLink(sb, 1);
            AppendPageLink(sb, startingPoint, "&laquo;");
            sb.AppendLine("&nbsp;&nbsp;&nbsp;");
        }
    }
    private void AppendTerminalLinks(StringBuilder sb, int linksAfterCurrent, int totalPages) {
        If (currentPage < linksAfterCurrent && linksAfterCurrent < totalPages) {
            sb.AppendLine("&nbsp;&nbsp;&nbsp;");
            AppendPageLink(sb, linksAfterCurrent+1)
            AppendPageLink(sb, totalPages)
        }

    }
}

I rewrote it in C# because that is easier for me (and also Sublime Text doesn't have a VB mode) so you'll have to convert the syntax back to VB.

The overall structure of this is not bad. I moved the extension logic to its own class. This is so that you can have private helper methods which I created a bunch of. I am a big fan of the extract-private-method refactoring. If you read through the public Generate method now you will see it's very easy to pick out what's going on, whereas before you had to pick through the syntax.

Private methods are your friends.

Please Note: I have neither checked the logic nor compiled it. There were a couple points where it seemed like your logic could be simplified so I did (the calculation for endPoint seems like it would always be linksAfterCurrent for example). You should double check everything but the refactorings of extract-class and extract-private-method is the key to cleaning this up.