views:

871

answers:

4

I have a html table that I reorder based on a CSV list of custom attribute values that I have for each table row. I am using the following function to do it:

for (var i = 0; i < arrCSV.length; i++)
{
  $('#' + tableId)
      .find('[fname = ' + arrCSV[i] + ']')
      .eq(0)
      .parents('tr')
      .eq(0)
      .appendTo('#' + tableId);
}

The table structure is:

<table>
<tr>
 <td fname='f1'>something here</td>
</tr>
<tr>
 <td fname='f2'>something here</td>
</tr>
</table>

The CSV could be something like this "f2, f1"

I find this is very very slow performing function. Any help in optimizing it is really appreciated.

EDIT: Based on the article at http://www.learningjquery.com/2009/03/43439-reasons-to-use-append-correctly, one can achieve the greatest boost in performance by calling append only once with the html concatenated string. Can someone help in using this technique to my problem? I am not sure how to go about getting the s HTML in the for loop and appending it once.

A: 

You may have to rethink your algorithm.

Without changing the algorithm, a slight optimization would be:

var $table = $("#" + tableId);

for (var i = 0; i < arrCSV.length; i++)
{
  $('[fname = ' + arrCSV[i] + ']:first',$table).closest('tr').appendTo($table);
}
Philippe Leybaert
A: 

Wait a second...

$('#' + tableId)

Get #myTable

.find('[fname = ' + arrCSV[i] + ']')

Find anything with an attribute of fname equal to i

.eq(0)

Give me the first item of the previous expression

.parents('tr')

Find the parents of type TR

.eq(0)

Give me the first in the previous expression

.appendTo('#' + tableId);

Add that TR to #myTable

Okay. Now that I've broken it down - are you duplicating a Table Row only? If so, the .append() isn't your problem, your choices of selectors is. To further compound my confusion here, the only tags with an attribute of fname are your TR's, so why are you going to their parents() and seeking out the first TR? You're basically asking for TR tags to be placed within TR tags - but that isn't what your Markup example shows.

Jonathan Sampson
Ahhh.. Sorry, My bad. The fname attribute is on the <td> and not the <tr>
A: 

I would suggest finding the elements as few times as possible. Store all the matching rows into a "hash" using the attribute value of interest as the key. Go through your CSV, pick the corresponding row out of the hash, push it into an array, then finally append the elements of the array to the table using the jQuery object previously found.

var table = $('#' + tableId);
var rowHash = {};
table.find('[fname]').each( function() {
    rowHash[$(this).attr('fname')] = $(this).closest('tr');
});
var rows = [];
for (var i = 0; i < arrCSV.length; ++i)
{
    var row = rowHash[arrCSV[i]];
    if (row) {
       rows.push(row);
    }
}
$(rows).appendTo(table);

EDIT: This seems like a slight improvement to my previous code where I was appending each row to the table as it was found. I tested on a table with 1000 rows and it seems to take about 1sec to sort a table that needs to be completely inverted.

tvanfosson
I might be missing something. Can you tell me which line in the code gets the HTML for the TRs? Thanks for the help!
It's not adding via HTML. It takes the actual elements and appends them. The $(rows).appendTo(table) line takes each element in the rows array and appends it in turn to the table. I think it's pretty quick. With 50-100 (maybe more) rows it seems instantaneous.
tvanfosson
Thanks. I was able to bring it down from 67 to 43milliseconds using the code above. I wish there was a way to optimize this further.
+1  A: 

If you want to append html only once (like that learningjquery.com article), try following:

$(document).ready(
function()
{
   var arrCSV = ['f2', 'f1'];
   var tableId = 'mainTable';
   var newTable = [];
   for (var i = 0; i < arrCSV.length; i++)
   {
      var row = $('#' + tableId)
                .find('[fname = ' + arrCSV[i] + ']')
                .eq(0)
                .parents('tr')
                .eq(0);

      newTable.push(row.html());
   }                    

   $('#' + tableId).html(newTable.join(''));
};
});

Live version: http://jsbin.com/uwipo Code: http://jsbin.com/uwipo/edit

Though I personally feel that you should profile your code first and see if it's append which is slow OR that 'find' method call. I am thinking that for a huge table, using 'find method' to find a custom attribute could be slow. But again, there is no point in any guesswork, profile the code and find it out.

If the 'find' method is slow, will it be possible to use id attribute on td instead of giving custom attribute.

e.g.

<table>
<tr>
   <td id='f1'>something here</td>
</tr>
<tr>
   <td id='f2'>something here</td>
</tr>
</table>

Then your code to find the parent row could be as simple as:

  ('#' + arrCsv[i]).parent('tr')

EDIT: As pointed out by tvanfosson, this code assumes that arrCSV contains attribute for all the rows. The final table will only contain those rows which are present in arrCSV. Also, this code does not copy 'thead', 'tfoot' section from the original table, though it should be easy to write code which does.

SolutionYogi
This assumes that every row in the table gets ordered by the CSV -- or that rows that don't match should be dropped. That may be ok, but you should be aware of that.
tvanfosson
Yes, you are right. I am going to edit my post to reflect that.
SolutionYogi
Can I rewrite without using find and using the custom attribute? Unfortunately, i cannot use the id in my case