2
\$\begingroup\$

This is my simple implementation on JS HTML table generator which generates a table based on an array of JS objects.

function Column(caption,position,isVisible,values){
  this.caption = caption;
  this.position = position;
  this.isVisible = isVisible;
  this.values = values;
}


Column.prototype.printValue = function (index){
      return  "<td>"+ this.values[index] +"</td>";
};


var columnArray = [                    
                    new Column("Id",0,true,[1,2,3]),
                    new Column("Name",1,true,["Tim","Jim","Tom"]),
                    new Column("Date",2,true,["27-03-2014","01-01-2014","11-04-2015"])
                  ];

function columnSorter(columnA,columnB){
  if(columnA.position < columnB.position)
    return -1;
  if(columnA.position > columnB.position)
    return 1;
  return 0;
}


columnArray.sort(columnSorter);


function generateTable(columnArray){
  var htmlString = "<table><thead><tr>";

  for(var col in columnArray){
    htmlString += "<th>"+columnArray[col].caption+"</th>";
  }

  htmlString += "<tfoot></tfoot>";

  htmlString += "<tbody>";
  for(var i=0;i<columnArray[0].values.length;i++){
    htmlString += "<tr>";
    for(col in columnArray){
      htmlString += columnArray[col].printValue(i);
    }
    htmlString += "</tr>";
  }
  htmlString +="</tbody>";

  htmlString += "</tr></thead>";
  return htmlString += "</table>";
}
\$\endgroup\$
  • \$\begingroup\$ is your table input format can be changed ? \$\endgroup\$ – paritosh Jul 30 '14 at 15:59
3
\$\begingroup\$

The advantage of having a Column class and separate generateTable function is that you separate the data (in Column objects) from the HTML display (in generateTable). But your printValue method goes against this logic. It mixes HTML code into the Column class. So it should probably be moved to the generateTable function.

Note that <thead> is supposed to contain one or more <tr> elements (unless it is empty).

It is usually preferable to build up an HTML DOM object using document.createElement and element.appendChild rather than a string. This avoids mistakes such as putting closing tags in the wrong place and helps to ensure that you are outputting valid HTML.

That can get quite verbose, but if you use a helper function such as the following:

function add(el, parent, html) {
// Appends a new or existing element with the specified HTML content to the parent
    if (typeof el == 'string') el = document.createElement(el);
    if (parent) parent.appendChild(el);
    if (html) el.innerHTML = html;
    return el;
}

... the generateTable function is then quite succinct and readable:

function generateTable(columnArray) {
    var table = add('table'),
        theadRow = add('tr', add('thead', table)),
        tbody = add('tbody', table),
        tfoot = add('tfoot', table),  // tfoot not used but include anyway
        tr;
    for (var col = 0; col < columnArray.length; col++) {
        add('th', theadRow, columnArray[col].caption);
    }
    for (var row = 0; row < columnArray[0].values.length; row++) {
        tr = add('tr', tbody);
        for (var col = 0; col < columnArray.length; col++) {
            add('td', tr, columnArray[col].values[row]);
        }
    }
    return table; // or add(table, document.body) to append it to the page
}

Note that you no longer have to worry about closing tags at all.

The next step might be to turn this into a full class which would let you generate a table and do stuff to it afterwards, for instance adding or removing a new column.

\$\endgroup\$
  • \$\begingroup\$ MDN claims <thead> should contain zero or more <tr> elements. \$\endgroup\$ – Schism Jul 30 '14 at 18:06
  • \$\begingroup\$ @Schism Thanks for the correction. But <thead> is not permitted to contain any other kind of element; and <th> elements are only permitted inside <tr> elements. So it must contain a tr element unless it is empty. (I've altered the text to clarify this.) \$\endgroup\$ – Stuart Jul 30 '14 at 18:09
4
\$\begingroup\$
  • Since position is a number, you can simplify your columnSorter function. It becomes a one-liner, so you could even do:

    columnArray.sort(function(a, b) { return a.position - b.position; });
    

    Note though that some people consider this less readable. It's definitely less explicit in what it does, so you may wish to explain it in a comment.

  • You're using a for..in construct. This will give unexpected results if I extend Array.prototype. Either use a regular for loop, or check for columnArray.hasOwnProperty(col) in each iteration.

  • Why add an empty <tfoot>?

  • You're closing your <thead> in the wrong spot. <tbody> and <tfoot> should be children of <table>, not children of <thead>, so close your <thead> earlier.

  • It doesn't make much sense to return x += y;. Either return x + y;, or do x += y; return x;.

\$\endgroup\$
  • \$\begingroup\$ Also, the order in for..in is arbitrary. \$\endgroup\$ – konijn Aug 5 '14 at 13:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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