Reputation: 4786
I've got a table that has a toggle to show/hide various rows that match certain criteria and I'm using the following JS to achieve what I want (JSFiddle);
function resetRows() {
var i = 1,
tds;
var start = new Date().getTime();
$('td').removeClass('even odd');
$.each($('tr'), function (key, index) {
// loop through each table row skipping the first row which will be the title
tds = $(this).find($('td'));
if ($(this).is(':visible') && key > 0) {
if (i % 2 === 0) {
tds.addClass('even');
} else {
tds.addClass('odd');
}
// find the .index class and reset it's value
$(this).find($('.index')).html(i);
i++;
}
});
var end = new Date().getTime();
var time = end - start;
console.log('time taken: ' + time);
}
$(function () {
// show/hide failed
$('#showHideFails').on('click', function () {
if ($('.failed').eq(0).is(':visible')) {
$('.failed, .ins').hide();
$('.shTxt').html('Show all');
resetRows();
} else {
$('.failed, .ins').show();
$('.shTxt').html('Hide failed');
resetRows();
}
});
});
My problem is that this code is executing very slowly. The js fiddle is relatively quick as my example table only has 15 rows. My actual table has 100 rows, and can be changed to show 1,000 rows.
I've added a console log to test how long it's executing and in the fiddle it's taking ~15ms to hide the rows and ~20-40ms to show the rows again.
On my real table with 100 rows this changes to ~300-450ms to hide and ~500-600ms to show. This figure more than doubles when trying 200 rows (1,200 and 2,00 respectively). When I tried with 1,000 rows my browser nearly crashed.
I tried changing my code so that instead of changing the class names of td
's it changed the class names of the tr
's as in theory this would mean less processing required for jQuery to handle, however this almost doubled the execution time so I reverted back to td
's
Can anyone tell me how to make my code more efficient? It's a little frustrating to get the functionality that I want, only to have it run very slowly.
Upvotes: 0
Views: 577
Reputation: 1282
Some code optimization suggestions:
usage of .find()
tds = $(this).find($('td'));
is the same as tds = $(this).find('td')
$('td')
causes jquery to search for all table cells in your whole HTML before doing anything else
But what you really want is: All cells for the current row only which can be done with $(this).find('td')
Remove "dead" code
Your row-iteration-function does nothing if you're in the header-row or if the row has already been hidden. At the moment you're searching for cells in that row even if it is hidden.
$.each($('tr'), function (key, index) {
if (false == $(this).is(':visible') || 0 == key) {
// skip hidden rows and header row
return;
}
// at this point we know that the current row is visible and not the header row
tds = $(this).find($('td'));
...
});
Reduce jQuery-selectors
As far as I can see the selectors are the most cost-intensive part here. While that's not a problem for small data it can effect large data like your table with 1000 rows and such. Try to re-use the selector-results where possible by using a variable.
Replace $(this) with a variable:
var currentRow = $(this);
currentRow.is(':visible');
currentRow.find('td');
...
These are my thoughts so far. See this jsfiddle for rewritten code: http://jsfiddle.net/5jVdh/3/
Upvotes: 1
Reputation: 74420
You should detach your table from DOM when using this kind of elements iteration. You could then show any message to user like a loader during task.
function resetRows($table) {
var $loader = $('<div id="loader"/>').insertBefore($table),
$tblContent = $table.detach(),
i = 1,
trs = $tblContent.find('tr'),
tds = $tblContent.find('td').removeClass('even odd');
var start = new Date().getTime();
$.each(trs, function (key, index) {
// loop through each table row skipping the first row which will be the title
var tdsRow = $(this).find('td');
if (!$(this).data('hidden') && key > 0) {
if (i % 2 === 0) {
tdsRow.addClass('even');
} else {
tdsRow.addClass('odd');
}
// find the .index class and reset it's value
$(this).find('.index').html(i);
i++;
console.log(i);
}
});
var end = new Date().getTime();
var time = end - start;
$loader.replaceWith($table);
console.log('time taken: ' + time);
}
$(function () {
// show/hide failed
$('#showHideFails').on('click', function () {
if ($('.failed').eq(0).is(':visible')) {
$('.failed, .ins').data('hidden',true).hide();
$('.shTxt').html('Show all');
resetRows($('#table'));
} else {
$('.failed, .ins').data('hidden',false).show();
$('.shTxt').html('Hide failed');
resetRows($('#table'));
}
});
});
Upvotes: 3