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've written a JavaScript table class to make it easy to display tabular data and allow the user to sort the data by clicking on the table headers.

However, when there are hundreds of rows, performance on the sorting is a bit sluggish. I would like to know what kind of improvements I can make. I suspect that my string concatenation methods in displayData may not be optimal.

var tableUtility = (function () {
    var fn = {};
    // This is a constructor function for an object that represents a table
    fn.DataTable = function (p_data, p_columnDataFields, p_tableId, p_displayCallback) {
        // This is expected to be an array of objects.
        var data = p_data;
        // This is expected to be an array of strings that represents the names of the fields to use for the column data.
        var columnDataFields = p_columnDataFields;
        // This is expected to be the HTML ID of the table in which the data will be displayed.
        // Note that this JavaScript will only affect the TBODY of the TABLE.  It is expected that the HTML already contains the THEAD with the column headers.
        var tableId = p_tableId;
        // This is expected to be either null or a function to execute after the table is displayed.
        var displayCallback = p_displayCallback;

        // Find the TBODY element of the TABLE and cache it
        var tbody = document.getElementById(tableId).getElementsByTagName('tbody')[0];

        // softData sorts the table's data.  fields is expected to be an array of strings with the names of the fields to sort by.  sort functions is expected
        // to be an array of functions describing how to sort each field.
        // Note that fields must be distinct, or weird things will happen.
        this.sortData = function (sortFields, sortFunctions) {
            if (sortFields.length != sortFunctions.length)
                throw new Error('fields and sortFunctions don\'t have the same length.');

            var builtInSorts = this.sortData.sorts;

            var sortFunction = function (x, y) {
                var fieldIndex = 0;
                for (var i = 0; i < sortFields.length; i++) {
                    var first = x[sortFields[i]];
                    var second = y[sortFields[i]];
                    var sortFunction = sortFunctions[i];

                    // This allows a calling function to reference a built-in sorting function.                    
                    if (typeof sortFunction === 'string')
                        result = builtInSorts[sortFunction](first, second);
                    else
                        result = sortFunction(first, second);

                    //var result = sortFunction(first, second);
                    if (result !== 0)
                        return result; // difference found
                }
                return 0; // no difference was found over any of the fields                
            };

            data.sort(sortFunction);
        }; // end sortData

        // This object contains some built-in sorting functions that will be used on multiple pages
        this.sortData.sorts = {
            alphaSort: function (x, y) {
                if (x.toString().toLowerCase() < y.toString().toLowerCase()) return -1;
                else if (y.toString().toLowerCase() < x.toString().toLowerCase()) return 1;
                else return 0;
            },
            numericSort: function (x, y) {
                return parseInt(x, 10) - parseInt(y, 10);
            }     
        } // end sortData.sorts

        this.displayData = function () {
            var numberOfFields = columnDataFields.length;
            var numberOfRows = data.length;
            var html = '';
            for (x in data) {
                var entry = data[x];
                var row = '<tr>';
                for (var i = 0; i < numberOfFields; i++) {
                    row += '<td>' + entry[columnDataFields[i]] + '</td>'
                }
                row += '</tr>'
                html += row;
            }
            //tbody.innerHTML = html; I tried to do this without jQuery, but kept getting "Unknown Runtime Error in IE7"
            $(tbody).html(html);
            $(tbody)
                .find('tr:even').addClass('EvenBar')
                .find('tr:odd').addClass('OddBar');

            // If a callback function was set, invoke it.            
            if (typeof displayCallback === 'function')
                displayCallback();
        } // end displayData        
    } // end DataTable Constructor Function
    return fn;
})(); // end tableUtility
share|improve this question
add comment

1 Answer 1

up vote 1 down vote accepted

This was originally a comment, but it got long.

  • The jQuery/innerHTML stuff is probably slowing it down, as it looks like you guessed. Tables in IE + innerHTML is broken. Instead, I'd use the DOM.

  • Reconstituting the entire table for each sort is bound to be slow. Since you are doing the innerHTML thing, the entire table needs to be rebuilt, rather than just moving some tablerow nodes around.

Most javascript table sort scripts take a different approach. The table is produced on the server or by a content writer in the normal way, and given a class like "sortable-table" for example, possibly with some columns having classes for type hinting or other metadata as well. Target tables with the appropriate classes and make them sortable. Sort tables by taking the tbody element and sorting its rows by removing and re-appending them, using the DOM.

Even if you want to create the table element programatically, you could use a similar approach from there on.

Recap:

  • Use the DOM, not jQuery.
  • Use the DOM, not innerHTML.
  • Consider letting the table be created in the usual way instead of through an API.
  • Use the DOM.
share|improve this answer
    
Pro tip: Use the DOM. –  Raynos Feb 8 '12 at 21:07
    
@Raynos essentially, yes ;) –  Dagg Feb 8 '12 at 21:09
    
On this line: $(tbody).html(html);, I had originally written tbody.innerHTML = html, but that doesn't work in IE. –  Rice Flour Cookies Feb 8 '12 at 21:14
1  
That's because innerHTML is bad. Bad. –  Raynos Feb 8 '12 at 21:19
1  
@RiceFlourCookies you did see the part of my answer that says "Use the DOM, not innerHTML," right? Or the "Tables in IE + innerHTML is broken" part? –  Dagg Feb 8 '12 at 21:32
show 1 more comment

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.