Populate a table from a database using JavaScript from a jQuery UI Dialog Box

Question

I'm returning to Web Development after a number of years working in other areas.

The below image shows what what I have so far looks like. I'm not 100% satisfied with the styling yet, but that's a separate question - I include it here mostly to show what this does. (There will eventually be more fields on the table).

The History dialog is a standard jQuery UI dialog that comes up when you hover over the History button. Each link refers to what's internally referred to as a "Master Schedule." The list of Master Schedules is retrieved through a stored procedure.

When you click on a link, it populates the table with details of that schedule (and, again, I'll eventually have more fields than are shown on the table right now).

The table and button are inside an UpdatePanel because I don't want to do a full page refresh every time they view a new item from the history.

Here's my Update Panel:

<asp:UpdatePanel ID="detailsTableUpdatePanel" runat="server">
    <ContentTemplate>
        <asp:Button ID="history" Text="History" runat="server" OnClick="history_Click" />
        <input id="masterIDInput" type="hidden" runat="server" />
        <div id="dialog" runat="server"></div>
        <p />
        <asp:Table ID="detailsTable" Visible="false" runat="server" />
    </ContentTemplate>
</asp:UpdatePanel>

Here's my JavaScript:

<script type="text/javascript">
    $(document).ready(AddHover)

    function AddHover()
    {
        $("#MainContent_history").hover(function () {
            $("#MainContent_dialog").html(<%= divContent %>);
            $("#MainContent_dialog").dialog({
                title: "History",
                close: function (ev, ui) {
                    $(this).hide();

                    // I was having an issue where you could only see the History dialog once - once you
                    // closed it once it would never show up again
                    // This (along with a few other hacks) works but it seems like there should be a better way to do this
                    AddHover();
                }
            })
        })
    }

    function bringUpDetails(masterID) {
        // Add the ID of the Master Schedule we're looking for to the hidden form input
        $("#MainContent_masterIDInput").val(masterID);
        // Simulate clicking on the button to call the button's event handler
        // I don't really like this - it seems like I should just do some kind of
        // AJAX call here
        $("#MainContent_history").trigger("click");
        // Same as previous AddHover() call
        AddHover();
    }
</script>

Here's my C# code behind:

public string divContent;

    protected void Page_Load(object sender, EventArgs e)
    {
        if (!Page.IsPostBack)
        {
            BuildDivContent();
        }
    }

    private void BuildDivContent()
    {
        // I'm using LINQ to SQL for now
        // In the final version I'll probably replace this with Entity Framework
        DataClasses1DataContext context = new DataClasses1DataContext();
        List<spGetMasterScheduleHistoryForClientResult> results = context.spGetMasterScheduleHistoryForClient(295615).ToList();

        StringBuilder sb = new StringBuilder();

        if (results.Any())
        {
            sb.Append("\"");
        }

        foreach (var result in results)
        {
            sb.AppendFormat("<a onclick='bringUpDetails({0})' href='#'>{1} - {2}</a><br/>", result.MasterScheduleID, result.EffectiveBeginDate, result.EffectiveEndDate);
        }

        if (results.Any())
        {
            sb.Append("\"");
        }

        divContent = sb.ToString();
    }

    protected void history_Click(object sender, EventArgs e)
    {
        // Figure out from the hidden form field which master
        int id =
            masterIDInput.Value != null && masterIDInput.Value.Trim().Length > 0 ?
                int.Parse(masterIDInput.Value) :
                1;

        DataClasses1DataContext context = new DataClasses1DataContext();
        spGetMasterScheduleByIDResult result = context.spGetMasterScheduleByID(id).First();

        TableRow headerRow = new TableRow();
        headerRow.Cells.Add(new TableCell()
        {
            Text = "<b>Begin Date</b>"
        });
        headerRow.Cells.Add(new TableCell()
        {
            Text = "<b>End Date</b>"
        });

        detailsTable.Rows.Add(headerRow);

        TableRow detailsRow = new TableRow();
        detailsRow.Cells.Add(new TableCell()
        {
            Text = result.EffectiveBeginDate.ToString()
        });

        detailsRow.Cells.Add(new TableCell()
        {
            Text = result.EffectiveEndDate?.ToString() ?? "N/A"
        });

        detailsTable.Rows.Add(detailsRow);

        detailsTable.Visible = true;
    }

And, finally, here's my CSS:

<!-- Yes, I do realize that this should probably go in a separate stylesheet -->
<style type="text/css">
    table, th, td {
        border: 1px solid black;
        min-width: 250px;
    }

    a:link {
        color: green
    }

    a:visited {
        color: red
    }

    a:hover {
        color: blue
    }

    a:active {
        color: blue;
    }
</style>

I have code comments over a few specific things I was concerned about. Also, I'm sure that there are plenty of ways that I could optimize this as well and was hoping that someone could give me feedback on my overall approach.

Answer 1

Only targeting the C# code behind

BuildDivContent()

• Instead of calling two times if (results.Any()) for appending " you should check once for if (!results.Any()) and if that is true, assign string.Empty" todivContent` and return early.
• If you can estimate how many chars the StringBuilder will contain at the end you can use the overloaded constructor which takes an initial capacity. This removes the need of the StringBuilder if its initial capicity isn't high enough to increase the internal data structure.
• Because you are allready using C# 6 you could take advantage of the string interpolation feature using the $ sign instead of using AppendFormat of the StringBuilder.

Implementing the mentioned points leads to

private void BuildDivContent()
{
    // I'm using LINQ to SQL for now
    // In the final version I'll probably replace this with Entity Framework
    DataClasses1DataContext context = new DataClasses1DataContext();
    List<spGetMasterScheduleHistoryForClientResult> results = context.spGetMasterScheduleHistoryForClient(295615).ToList();

    if (!results.Any())
    {
        divContent = string.Empty;
        return; 
    }

    StringBuilder sb = new StringBuilder("\"", 1024);

    foreach (var result in results)
    {
        sb.Append($"<a onclick='bringUpDetails({result.MasterScheduleID})' href='#'>{result.EffectiveBeginDate} - {result.EffectiveEndDate}</a><br/>");
    }

    sb.Append("\"");

    divContent = sb.ToString();
}

history_Click()

Like any method this method should be named using PascalCase casing. In addition, please don't use underscores in method names.

The evaluation of int id should be done more idiomatic by using string.IsNullOrWhitheSpace() like so

int id = string.IsNullOrWhiteSpace(masterIDInput.Value) ? 1 : int.Parse(masterIDInput.Value);