Meet Results Displayer

I decided to dive into the world of JS/JQuery by trying to build a simple web app to display all of my various meet results in an easy-to-view manner. For this stage of the review, all it does is generate a table of all the meets and log some stuff to the console when you click on it.

The app gets its data as JSON in a form like:

[{"Date": "MM/DD/YYYY", "Name": "Some Event", "Sport": "Run", "Events": [...]},
 {"Date": "MM/DD/YYYY", "Name": "Some Other Event", "Sport": "Swim", "Events": [...]},
 ...
 ]

and displays them in a table like:

Additionally, I'm storing the "Events" part in each cell so that when I click on a cell, I can eventually do something interesting with the data.

Is there anything I'm doing that violates some key language principles? Is what I'm doing with events too hacky? Is there a simpler way to build up the table? All comments welcome.

```

$(document).ready(function() {
$.ajax({url: 'http://localhost:8080/all_events',
cache: false,
type: 'GET',
dataType: 'jsonp',
crossDomain: true,
success: function(data) {
var tbl_body = document.createElement("tbody");
$.each(data, function(idx, elem) {
var tbl_row = tbl_body.insertRow();
tbl_row.className = idx%2 == 0 ? "even" : "odd";

tbl_row.insertCell().
appendChild(document.createTextNode(elem.Date));

event = document.createTextNode(elem.Name);
empty = document.createTextNode('');

if (elem.Sport == 'Run') {
var cell = tbl_row.insertCell();
cell.className = 'Event';
cell.events = elem.Events;
cell.appendChild(event);
tbl_row.insertCell().appendChild(empty);
}
else {

High Level Notes

My most substantial suggestion is to avoid mixing data logic and view code. Transform your data, as pure data, into the form you would like it to be so that your view code can treat it uniformly. Then your view will be more like a "dumb" mail-merge template, even if you decide to keep using jquery to build it up programmatically. A rewrite should clarify what I mean:

This will take each event that the ajax returns and transform it into data for both of the row cells:

function eventToRowData(event) {
    var nullCell = {"Name": '', "Sport":null, "Events": null},
        isRun = event.Sport == 'Run';
    return {
        "Date": event.Date,
        "RunCell": isRun ? event : nullCell,
        "SwimCell": isRun ? nullCell : event
    };
}

Now all the if...else logic vanishes from your view code:

function buildTable(data) {
    var rowData = data.map(eventToRowData),  
        tableBody = document.createElement("tbody");

    rowData.forEach(makeRow);
    $("#my_events").append(tableBody);

    function makeRow(rowDatum) {
        var row = tableBody.insertRow();
        row.insertCell().appendChild(document.createTextNode(rowDatum.Date));
        addEventCell(rowDatum.RunCell);
        addEventCell(rowDatum.SwimCell);

        function addEventCell(cellData) {
            var cell = row.insertCell();
            cell.className = cellData.Sport ? 'Event' : '';
            cell.Events = cellData.Events;
            cell.appendChild(document.createTextNode(cellData.Name));
        }    
    }
}

See below for full working snippet.

Other Notes

• To mitigate nesting, avoid defining things like success callbacks inline, unless they are so short they can fit on a single line. This will make both the callback itself and whatever is calling it (in your case, an ajax options object) more readable.

• Table row striping and hover highlighting belong in css. As a rule, most view things that can be done in css should.

• Consider ditching jquery. The only good reason to have it in this code is simplifying your ajax call. You could use zepto instead, or do it raw, and save some KB. See this link for more.

• Consider using a light template library to clean up the view code which builds the table.

• Consider using mithril.js (an extremely light library with the same philosophy as React) if this becomes more complex, or if you are interested in taking the separation of data view even further.

• I didn't fix all the trivial things like naming conventions. js is camelCase, css is dash-case by convention.

• Finally, this may not be relevant, since I get the sense you are just doing this to learn, but unless you have more complex, interactive features, I think server side code rendering a static site is more appropriate for this use case.

var sampleData = [{
"Date": "MM/DD/YYYY",
"Name": "Some Event",
"Sport": "Run",
"Events": [1, 2]
}, {
"Date": "MM/DD/YYYY",
"Name": "Some Other Event",
"Sport": "Swim",
"Events": [1, 2]
}, {
"Date": "MM/DD/YYYY",
"Name": "Another Event",
"Sport": "Swim",
"Events": [1, 2]
}, ]

// Rather than work with data in an inconvenient format, change
// it to a format that suits our view code.
function eventToRowData(event) {
var nullCell = {"Name": '', "Sport":null, "Events": null},
isRun = event.Sport == 'Run';
return {
"Date": event.Date,
"RunCell": isRun ? event : nullCell,
"SwimCell": isRun ? nullCell : event
};
}

function buildTable(data) {
var rowData = data.map(eventToRowData),
tableBody = document.createElement("tbody");

rowData.forEach(makeRow);
$("#my_events").append(tableBody);

function makeRow(rowDatum) {
var row = tableBody.insertRow();
row.insertCell().appendChild(document.createTextNode(rowDatum.Date));
addEventCell(rowDatum.RunCell);
addEventCell(rowDatum.SwimCell);

function addEventCell(cellData) {
var cell = row.insertCell();
cell.className = cellData.Sport ? 'Event' : '';
cell.Events = cellData.Events;
cell.appendChild(document.createTextNode(cellData.Name));
}
}
}

$(document).ready(buildTable(sampleData))
table, th, td {
border: 1px solid black;
}
td {
text-align: center;
vertical-align: middle;
padding: 5px;
}
#my_events tbody tr:nth-child(even) {
background-color: #ccc;
}
td.Event:hover { background: lime }




Date
Run
Swim




StackExchange Code Review Q#110148, answer score: 5