1
\$\begingroup\$

In the following code, I've just parsed an JSON object called responseObject from an AJAX call, one of whose members is an array of objects called theArray. For each object in that array, I'm creating a row in a table. Each object contains three strings named code, author, date, and module. Each row gets three columns, one for each of these three strings.

Rather than doing the following four times (once for each property):

var codeTd = document.createElement("td");
codeTd.innerHTML = responseObject.theArray[i].code;

I'm doing it like this:

var codeTd, authorTd, dateTd, moduleTd;
(codeTd = document.createElement("td")).innerHTML = responseObject.theArray[i].code;
(authorTd = document.createElement("td")).innerHTML = responseObject.theArray[i].author;
(dateTd = document.createElement("td")).innerHTML = responseObject.theArray[i].date;
(moduleTd = document.createElement("td")).innerHTML = responseObject.theArray[i].module;

I've changed some of the names around, but you get the idea. Is this an okay idiom? It seems to save repetitiveness without sacrificing much in readability once you understand what it's doing (for me, it's easier to read) and it helps the code be DRY.

Also I was wondering if anyone had seen this used before.

An alternative that I've been thinking of would be:

var keys = ["code", "author", "date", "mobile"];
for(var j = 0; j < keys.length; j++) {
    var td = document.createElement("td");
    td.innerHTML = responseObject.theArray[i][keys[j]];
    // append the td to the tr etc.
}

I guess that would work as long as the tds are not needed later for other things, but I'm not sure if it's actually more readable. Maybe more familiar though.

Now that I think about it, the second method has the advantage that you can more easily append them all to the tr, so it seems like the better choice. I'd still like to hear your opinions on the first (or second) method though.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Method two seems much more flexible for expansion. If you add columns in the future, you won't have to modify your code.

// Loop through row data
for(var i = 0; i < responseObject.theArray.length; i++) {
    var colObj = repsonseObject.theArray[i];

    // Loop through col data
    for(var colKey in colObj) {

        // Safety check, probably not needed
        if(colObj.hasOwnProperty(colKey)) {

            // Create td            
            var td = document.createElement("td");
            td.innerHTML = colObj[colKey];

            // Append to tr, etc.
        }
    }
}

If the number of columns was large, you could increase performance by appending outside of the loops via Document Fragments.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ I like your approach but imho your code has a bit too much vertical whitespace... and it's also perhaps a bit overzealous with regard to comments. +1 nevertheless :) \$\endgroup\$
    – Schism
    Commented Aug 22, 2014 at 15:58
  • \$\begingroup\$ Thanks @Schism. For production code, certainly! For a code review answer, I tend to comment a bit. \$\endgroup\$
    – lebolo
    Commented Aug 22, 2014 at 16:01
  • \$\begingroup\$ I agree with both of you. Thank you for the answer, lebolo. \$\endgroup\$ Commented Aug 22, 2014 at 16:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.