Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

The following code sorts an HTML table with JavaScript (without using any external libraries like jQuery). Are there any shortcomings or possible improvements I could make?

<html>

<head>
    <meta http-equiv="content-type" content="text/html;charset=Windows-1252">
    <script type="text/javascript">
        var people, asc1 = 1,
            asc2 = 1,
            asc3 = 1;
        window.onload = function () {
            people = document.getElementById("people");
        }

        function sort_table(tbody, col, asc) {
            var rows = tbody.rows,
                rlen = rows.length,
                arr = new Array(),
                i, j, cells, clen;
            // fill the array with values from the table
            for (i = 0; i < rlen; i++) {
                cells = rows[i].cells;
                clen = cells.length;
                arr[i] = new Array();
                for (j = 0; j < clen; j++) {
                    arr[i][j] = cells[j].innerHTML;
                }
            }
            // sort the array by the specified column number (col) and order (asc)
            arr.sort(function (a, b) {
                return (a[col] == b[col]) ? 0 : ((a[col] > b[col]) ? asc : -1 * asc);
            });
            // replace existing rows with new rows created from the sorted array
            for (i = 0; i < rlen; i++) {
                rows[i].innerHTML = "<td>" + arr[i].join("</td><td>") + "</td>";
            }
        }
    </script>
    <style type="text/css">
        table {
            border-collapse: collapse;
            border: none;
        }
        th,
        td {
            border: 1px solid black;
            padding: 4px 16px;
            font-family: Times New Roman;
            font-size: 24px;
            text-align: left;
        }
        th {
            background-color: #C8C8C8;
            cursor: pointer;
        }
    </style>
</head>

<body>
    <table>
        <thead>
            <tr>
                <th onclick="sort_table(people, 0, asc1); asc1 *= -1; asc2 = 1; asc3 = 1;">Name</th>
                <th onclick="sort_table(people, 1, asc2); asc2 *= -1; asc3 = 1; asc1 = 1;">Surname</th>
                <th onclick="sort_table(people, 2, asc3); asc3 *= -1; asc1 = 1; asc2 = 1;">Age</th>
            </tr>
        </thead>
        <tbody id="people">
            <tr>
                <td>Raja</td>
                <td>Dey</td>
                <td>18</td>
            </tr>
            <tr>
                <td>Mamata</td>
                <td>Sharma</td>
                <td>20</td>
            </tr>
            <tr>
                <td>Avijit</td>
                <td>Sharma</td>
                <td>21</td>
            </tr>
            <tr>
                <td>Sharanya</td>
                <td>Dutta</td>
                <td>26</td>
            </tr>
            <tr>
                <td>Nabin</td>
                <td>Roy</td>
                <td>27</td>
            </tr>
        </tbody>
    </table>
</body>

</html>

share|improve this question

migrated from stackoverflow.com Dec 17 '13 at 22:19

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

5  
Is the code otherwise working? If you're looking for just feedback, you should post to Code Review instead. – Juhana Dec 17 '13 at 9:37
    
Why not use Jquery. You can use tablesorter plugin. tablesorter.com/docs/example-pager.html – Anup Dec 17 '13 at 10:14
    
His code is working (it only had a misconfiguration on jsfiddle: jsfiddle ) and he is asking for performance tips. @Anup : (without any external library like JQuery) – ProGM Dec 17 '13 at 10:20
up vote 7 down vote accepted

To speed up the sorting you first have to find what is consuming time. In your case the slower part of your code is:

for(i = 0; i < rlen; i++){
    rows[i].innerHTML = "<td>"+arr[i].join("</td><td>")+"</td>";
}

The reason is that the DOM elaboration is time consuming for the browser.

Here's an updated version that minimizes the access to the DOM:

function sort_table(tbody, col, asc){
    var rows = tbody.rows, rlen = rows.length, arr = new Array(), i, j, cells, clen;
    // fill the array with values from the table
    for(i = 0; i < rlen; i++){
    cells = rows[i].cells;
    clen = cells.length;
    arr[i] = new Array();
        for(j = 0; j < clen; j++){
        arr[i][j] = cells[j].innerHTML;
        }
    }
    // sort the array by the specified column number (col) and order (asc)
    arr.sort(function(a, b){
        return (a[col] == b[col]) ? 0 : ((a[col] > b[col]) ? asc : -1*asc);
    });
    for(i = 0; i < rlen; i++){
        arr[i] = "<td>"+arr[i].join("</td><td>")+"</td>";
    }
    tbody.innerHTML = "<tr>"+arr.join("</tr><tr>")+"</tr>";
}

Proof: http://jsperf.com/table-sorting-stack-overflow

share|improve this answer

Here is the pure JS for sorting table data. I used first column which holds numbers. You have to modify column index and condition statement as per your requirements. I hope that solves your problem...

    // Table data sorting starts....
    function sortData() {
        // Read table body node.
        var tableData = document.getElementById('data_table').getElementsByTagName('tbody').item(0);

        // Read table row nodes.
        var rowData = tableData.getElementsByTagName('tr'); 

        for(var i = 0; i < rowData.length - 1; i++) {
            for(var j = 0; j < rowData.length - (i + 1); j++) {

                //Swap row nodes if short condition matches
                if(parseInt(rowData.item(j).getElementsByTagName('td').item(0).innerHTML) > parseInt(rowData.item(j+1).getElementsByTagName('td').item(0).innerHTML)) {
                    tableData.insertBefore(rowData.item(j+1),rowData.item(j));
                }
            }
        }
    }
    // Table data sorting ends....

HTML Table:

<table id="data_table" width="200" border="1">
    <tbody>
    <tr>
        <td width="100">01</td>
        <td>aaa</td>
    </tr>
    <tr>
        <td>04</td>
        <td>ddd</td>
    </tr>
    <tr>
        <td>05</td>
        <td>eee</td>
    </tr>
    <tr>
        <td>03</td>
        <td>ccc</td>
    </tr>
    <tr>
        <td>02</td>
        <td>bbb</td>
    </tr>
    <tr>
        <td>06</td>
        <td>fff</td>
    </tr>
    </tbody>
</table>

HTML Table: Sorted

<table id="data_table" width="200" border="1">
    <tbody>
    <tr>
        <td width="100">01</td>
        <td>aaa</td>
    </tr>
    <tr>
        <td>02</td>
        <td>bbb</td>
    </tr>
    <tr>
        <td>03</td>
        <td>ccc</td>
    </tr>
    <tr>
        <td>04</td>
        <td>ddd</td>
    </tr>
    <tr>
        <td>05</td>
        <td>eee</td>
    </tr>
    <tr>
        <td>06</td>
        <td>fff</td>
    </tr>
    </tbody>
</table>

Here is demo.

share|improve this answer
    
This whole answer is all jquery. Although I think it's great you answered, the OP was asking specifically for non jquery solutions. – dreza Dec 21 '13 at 5:33
    
Here you go. Pure JS code for sorting HTML nodes. You can see the demo on given link. – BwithLove Dec 22 '13 at 11:18

I ran into some trouble with ProGM's solution.

  1. It didn't take numerical sorting into account.
  2. IE did NOT like setting this line: tbody.innerHTML = "<tr>"+arr.join("</tr><tr>")+"</tr>";
  3. It would destroy any attributes assigned to any part of the table (css class names and etc)

I have made adjustments:

function sort_table(tbody, col, asc)
{
    var rows = tbody.rows;
    var rlen = rows.length;
    var arr = new Array();
    var i, j, cells, clen;
    // fill the array with values from the table
    for(i = 0; i < rlen; i++)
    {
        cells = rows[i].cells;
        clen = cells.length;
        arr[i] = new Array();
      for(j = 0; j < clen; j++) { arr[i][j] = cells[j].innerHTML; }
    }
    // sort the array by the specified column number (col) and order (asc)
    arr.sort(function(a, b)
    {
        var retval=0;
        var fA=parseFloat(a[col]);
        var fB=parseFloat(b[col]);
        if(a[col] != b[col])
        {
            if((fA==a[col]) && (fB==b[col]) ){ retval=( fA > fB ) ? asc : -1*asc; } //numerical
            else { retval=(a[col] > b[col]) ? asc : -1*asc;}
        }
        return retval;      
    });
    for(var rowidx=0;rowidx<rlen;rowidx++)
    {
        for(var colidx=0;colidx<arr[rowidx].length;colidx++){ tbody.rows[rowidx].cells[colidx].innerHTML=arr[rowidx][colidx]; }
    }
}

To address point #1, I added in the calls to parseFloat() and then compared the result the original value (You change to checking if it produces NaN instead). If both values are numeric, they compared via numerical preference and not by their string versions.

For points #2 and #3, I solved it via the same set of code. Instead of destroying the whole table body by setting the innerHTML, I change the individual cell contents to the new sorted values:

for(var rowidx=0;rowidx<rlen;rowidx++)
{
    for(var colidx=0;colidx<arr[rowidx].length;colidx++)
    { 
        tbody.rows[rowidx].cells[colidx].innerHTML=arr[rowidx][colidx]; 
    }
}

That only works if yours all have the same number of columns, but if you do not, sorting is much more complex anyway.

share|improve this answer
    
will you explain the adjustments please and how they overcome the issues that you found? – Malachi Nov 10 '14 at 17:12
1  
Can do, I also found a mistake in mine. – Plater Nov 11 '14 at 13:51

Here is an improved version based on Plater's anwser. Just in case there is DOM elements inside of td tags, the code gets the last child of the cell and get the value, and apply a customized filter to it.

Note: The customized filter here is just an example, and also you don't need to get the last child of the td tags, and it can be any child of your use case.

function sortTable(tbody, col, asc) {
  var rows = tbody.rows;
  var rlen = rows.length;
  var arr = new Array();
  var i, j, cells, clen;
  /* fill the array with values from the table */
  for (i = 0; i < rlen; i++) {
    cells = rows[i].cells;
    clen = cells.length;
    arr[i] = new Array();
    for (j = 0; j < clen; j++) {
      arr[i][j] = cells[j].innerHTML;
    }
  }
  /* sort the array by the specified column number (col) and order (asc) */
  arr.sort(function(a, b) {
    var retval = 0;
    var aVal = getDomValue(a[col]);
    var bVal = getDomValue(b[col]);
    var fA = parseFloat(aVal);
    var fB = parseFloat(bVal);
    if (aVal != bVal) {

      if ((fA == aVal) && (fB == bVal)) {
        retval = (fA > fB) ? asc : -1 * asc;
      } // Numerical
      else {
        retval = (aVal > bVal) ? asc : -1 * asc;
      } // String
    }
    return retval;
  });
  /* fill the table with sorted values */
  for (var rowidx = 0; rowidx < rlen; rowidx++) {
    for (var colidx = 0; colidx < arr[rowidx].length; colidx++) {
      tbody.rows[rowidx].cells[colidx].innerHTML = arr[rowidx][colidx];
    }
  }

}

function getDomValue(domString) {
  var value;
  var parser = new DOMParser();
  var element = parser.parseFromString(domString, 'text/xml');
  var content = element.lastChild.innerHTML;

  // Custom filter just in case if there is elements inside of the td.
  // I made two filters to standardize numbers like '$20,000', also I
  // treat whereas ' ' space as '0'
  if (content === ' ') content = content.replace(' ', '0');
  if (content.indexOf('$') !== -1) {
    content = content.replace('$', '');
    content = content.replace(',', '');
  }
  value = isNaN(parseFloat(content)) ? content : parseFloat(content);
  return value;
}

share|improve this answer

protected by Jamal Nov 6 '15 at 20:18

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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