Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am trying to append data to a table based on the ajax returned data.

My codes work but it seems ugly. I was wondering if anyone here can help me to simplfy it. Thanks a lot!

    ajax.callback=function(data){
          for (var i=0; i<dataObj[0].data.length; i++){
              var td=document.createElement('td');
              td.innerHTML=dataObj[0].data[i].ID;
              var td2=document.createElement('td');
              td2.innerHTML=dataObj[0].data[i].test;
              var td3=document.createElement('td');
              td3.innerHTML=dataObj[0].data[i].year;
              var td4=document.createElement('td');
              td4.innerHTML=dataObj[0].data[i].code;
              var td5=document.createElement('td');
              td5.innerHTML=dataObj[0].data[i].Label;
              var td6=document.createElement('td');
              td6.innerHTML=dataObj[0].data[i].contents;
              var td7=document.createElement('td');
              td7.innerHTML=dataObj[0].data[i].test;

              var tr=document.createElement('tr');
              tr.appendChild(td);
              tr.appendChild(td2);
              tr.appendChild(td3);
              tr.appendChild(td4);
              tr.appendChild(td5);
              tr.appendChild(td6);
              tr.appendChild(td7);


              $('#Table').append(tr);

           }
    }
share|improve this question

migrated from stackoverflow.com Oct 8 '12 at 14:56

This question came from our site for professional and enthusiast programmers.

    
Store your tr's as one dom fragment and do not append them until after the for loop. Other than that, it's not going to be less ugly without using a templating system or having your server return html. –  Kevin B Oct 4 '12 at 22:13
    
Should you be using data and not dataObj? –  Larry Battle Oct 8 '12 at 15:27

3 Answers 3

up vote 5 down vote accepted

You could do:

ajax.callback=function(data){
    var elements = ["ID", "test", "year", "code", "Label", "contents", "test"];

    for (var i=0; i<dataObj[0].data.length; i++){
        var td;
        var tr=document.createElement('tr');

        for (var j=0; j < elements.length; ++j){
            td = document.createElement('td');
            td.innerHTML=dataObj[0].data[i][elements[j]];
            tr.appendChild(td);
        }

        $('#Table').append(tr);
    }
}
share|improve this answer
    
document.createElement and innerHtml is not very jQuery'ish. Why not just td = $('<td>').html(dataObj[0].data[i][elements[j]); ? –  Klaus Byskov Hoffmann Oct 4 '12 at 22:16
1  
I'm not really familiar enough with jQuery to put stuff like that in. I was just showing him a way he could refactor his js in a js sense. If that's valid jquery, then by all means, make an answer with that instead. –  CrazyCasta Oct 4 '12 at 22:23
    
Thanks so much, your code works except dataObj[0].data[i].elements[j] doesn't equal dataObj[0].data[i].ID dataObj[0].data[i].elements[j] give me can't read property of 0 errro +1 though –  Rouge Oct 4 '12 at 22:35
    
@Rouge I made a little mistake in closing the [ bracket, so it may have been unclear. It's not .elements[j], it's [elements[j]]. –  CrazyCasta Oct 4 '12 at 22:37
    
awesome ! thanks a lot! –  Rouge Oct 4 '12 at 22:41

One (however not the most performant) of the more readable ways of doing this:

var tr = '<tr>\
    <td>{id}</td>\
    <td>{year}</td>\
    ...
    <td>{test}</td>\
</tr>';

tr = tr.replace(/{id}/, dataObj[0].data[i].ID)
    .replace(/{year}/, dataObj[0].data[i].year)
    .replace(/{test}/, dataObj[0].data[i].test);

$('#Table').append(tr);

Or use some templating engine.

share|improve this answer

You could use

ajax.callback=function(data){
    var keys=["ID","test","year","code","Label","contents","test"],
        tr=document.createElement('tr'),
        td=document.createElement('td');
    for (var i=0, li=dataObj[0].data.length; i<li; i++){
        var newTr=tr.clonenode(false);
        for(var j=0, lj=keys.length; j<lj; j++){
            var newTd=td.cloneNode(false);
            newTd.innerHTML=dataObj[0].data[i][keys[j]];
            newTr.appendChild(newTd);
        }
        document.getElementById('Table').appendChild(newTr);
    }
}

It's pretty much like CrazyCasta's answer, but cloning nodes is faster than creating them.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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