How to make make this code clearer?

Question

I am sorry if it is a simple problem for you.But i am really struggling with this problem

In our application we have plenty of ajax calls in each and every page. We are using jQuery ajax. Presently what we are doing is

When we get the result from like myresults, we are passing to a function populate where we are building the result.

function populate(myresults)
{        
    var str='<table>';       
    for(var i in myresults){
        str+='<tr>';
        str+=myresults[i].name;
        str+='</tr>';    
    }
    str+='</table>';

    $('#divId').html(str);
}

Not exactly we are building tables in all places. The code is working perfectly, but I think writing of code like this is not the right approach. How can I can beautify my code. I can use jQuery or javascript.

Can you use spaces between sentences (in addition to javascript and jquery)? — Aleks G, Apr 29 '13 at 11:30
This isn't really a question and will probably be voted off. — MMM, Apr 29 '13 at 11:30
A simple improvment would be to use a template engine, like handlebars. Or you can take it one step further, with knockoutjs, angularjs or similar. — NilsH, Apr 29 '13 at 11:30
Have you tried KnockoutJS for writing the templates and taking care of the binding? — gnz, Apr 29 '13 at 11:31
Try using a templating engine like [jsrender](https://github.com/BorisMoore/jsrender) — Arun P Johny, Apr 29 '13 at 11:31
This question is more suited for http://codereview.stackexchange.com/ Short answer: escape data to avoid XSS, use a templating library. — Alexey Lebedev, Apr 29 '13 at 11:32
[demo](http://plnkr.co/edit/B9W1wlgCF9WbbEFTxQrF?p=preview) using jsrender — Arun P Johny, Apr 29 '13 at 11:42
@PSR one of the problem with your design is, if later you want to change the markup it becomes very difficult. If you use a template like jsrender then the view is separated from the data and is added as a markup in the html which can be easily modified. Then you can club both the template and data to create the desired markup — Arun P Johny, Apr 29 '13 at 11:48
you have to include the library from https://github.com/BorisMoore/jsrender download the jsrender.js file — Arun P Johny, Apr 29 '13 at 11:55
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/29108/discussion-between-arun-p-johny-and-psr) — Arun P Johny, Apr 29 '13 at 11:55

score 1 · Accepted Answer · edited May 23 '17 at 11:43

1

You should try to look for TemplateEngine - it could allow to decrease code count and make it more clear. What Javascript Template Engines you recommend?

edited May 23 '17 at 11:43

Community

1
1

answered Apr 29 '13 at 11:31

Dmitry Volokh

1,630
16
28

Derek Henderson · Answer 2 · 2013-04-29T12:05:55.920

1

You should run your code through a linting engine like JSLint or JSHint and you should familiarize yourself with good practice.

Here's one way (of more than one possible solution) to optimize your code:

function populate(myresults) {
    var table = $(document.createElement('table'));       

    $(myresults).each(function (i) {
        var row = $(document.createElement('tr')),
            cell = $(document.createElement('td')).text(myresults[i].name);
        row.append(cell);
        table.append(row);
    });

    $('#divId').append(table);
}

edited Apr 29 '13 at 12:05

answered Apr 29 '13 at 11:38

Derek Henderson

9,388
4
42
71

`var table = $(document.createElement('table'));` there are some typos on the code... missing the `)` closing in the jquery selectors. – a--m Apr 29 '13 at 12:05

score 0 · Answer 3 · answered Apr 29 '13 at 11:36

Maybe, creating a new DOM element more correct ?

Like this:

function populate(myresults)
  {        
   var str= $('<table />)';       
      for(var i in myresults){
        str.append($('<td />').text(myresults[i].name).appendTo($('<tr />')));
      }
    $('#divId').empty().append(str);
  }

score 0 · Answer 4 · answered Apr 29 '13 at 11:51

You could do it this way:

function populate(myResults)
{        
    var tab = $('<table />');       
    for(var i in myResults){
      if (myResults.hasOwnProperty(i)) {
          var tr = $('<tr />');
          tr.append($('<td /'>, {text: i.name}));
          tab.append(tr);
      }
    }
    $('#divId').append(tab);
}

Using a template engine as suggested by the others would be preferable as it's a better, more maintainable approach.

How to make make this code clearer?

4 Answers4