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

I want to perform some operations on the gridview (asp.net 4.0). So, to achieve this i have written a jquery function which i am calling on pageload but, because this function takes some time for execution my grid performance degrades (IE 8). Can i optimize it someway?

     $.each($("#divTab2GridInquiries").find("tr").not(":first"), function () {
            var tr = $(this);
            var val = tr.find("input[id*='hdnLineStatus']").val();
            var btnDelete = tr.find("div[id='divBtnDelete']");
            var btnTobeDeleted = tr.find("div[id='divBtnTobeDeleted']");
            if (val == "N") {
                btnDelete.hide();
                btnTobeDeleted.hide();
            }
            if (val == "S") {
                tr.css("background-color", "#99FF99");
                tr.find("input").css("background-color", "#99FF99");
                btnDelete.show();
                btnTobeDeleted.hide();
            }

            if (val == "D") {
                tr.css("background-color", "#FFFF99");
                tr.find("input").css("background-color", "#FFFF99");
                btnDelete.show();
                btnTobeDeleted.hide();
            }

            //From user rights 
            if ($("input[id*='hdnTab2ShowDelete']").val() != "Y") {
                btnTobeDeleted.hide();
                //btnDelete.show();
            }
        });
share|improve this question
4  
Not an optimisation, but two comments: (1) You seem to have multiple elements with the same id, which is invalid html and should be avoided - use classes; (2) The first line will work but is using the "wrong" sort of .each(), so I'd say: $("#divTab2GridInquiries").find("tr").not(":first").each(function () { – nnnnnn Jul 20 '12 at 3:34
Do you mean to always hide btnTobeDeleted.hide();? – Bill Barry Jul 21 '12 at 2:26

migrated from stackoverflow.com Jul 20 '12 at 13:05

3 Answers

up vote 3 down vote accepted

Use a single selector instead of "find" and "not." Leave the find, this is faster in every browser except opera (see comments).

For the 2nd part of the selector, you can use the "sibling" combinator ~ to grab everthing except its first operand, and the :first-child pseudoclass selector to get the first child, giving you the same set of elements without using several jQuery methods. This is faster than using not(':first') in all browsers, and faster than a single selector (e.g. not using find either) in all browsers except Opera (which maintains its native-selector edge). See this test.

Note: #someTable tr will also return tr elements from a nested table. You really want to target the direct row descendants of the table. But don't forget about tbody, which is a required element. So this probably should be "#divTab2GridInquiries > tbody > tr:first-child ~ tr". But that is a mouthful... and it's really slow. If you have no nested tables it will work fine as coded below.

$.each($("#divTab2GridInquiries").find("tr:first-child ~ tr"), function () {      
    var tr = $(this);

Not sure what you're doing here - the selector is using a wildcard match, but val only operates against the first element in a selection set. Can you target this element more specifically? In any event, instead of wildcard matching the id, add a class and select on that. Classes are much faster than substring matching attributes.

        //var val = tr.find("input[id*='hdnLineStatus']").val();
        var val = tr.find(".hdnLineStatus").val();

IDs are supposed to be unique. I'm not sure why you would have to target it this way. But using an attribute selector like this will definitely be slower than a regular ID or class selector. If these ids are really unique then just use #divBtnDelete. I suspect that they aren't and you're creating invalid html. Get rid of the ID an add a class.

        // var btnDelete = tr.find("div[id='divBtnDelete']");
        var btnDelete = tr.find(".divBtnDelete");

        //var btnTobeDeleted = tr.find("div[id='divBtnTobeDeleted']");
        var btnTobeDeleted = tr.find(".divBtnTobeDeleted");

This set of ifs should be a switch, but that's probably not slowing you down nearly as much as the selectors.

        if (val == "N") {
            btnDelete.hide();
            btnTobeDeleted.hide();
        }
        if (val == "S") {
            tr.css("background-color", "#99FF99");h
            tr.find("input").css("background-color", "#99FF99");
            btnDelete.show();
            btnTobeDeleted.hide();
        }

        if (val == "D") {
            tr.css("background-color", "#FFFF99");
            tr.find("input").css("background-color", "#FFFF99");
            btnDelete.show();
            btnTobeDeleted.hide();
        }

Use a class again.

        //From user rights 
        //if ($("input[id*='hdnTab2ShowDelete']").val() != "Y") {
        if ($(".hdnTab2ShowDelete").val() != "Y") {

            btnTobeDeleted.hide();
            //btnDelete.show();
        }
    });
share|improve this answer
thanx for detailed explanation. I found what was slowing down foreach loop. my last condition : $("input[id*='hdnTab2ShowDelete']").val()!='Y' because while scanning each row of the table it was accessing outside hidden field. I fixed it – Tuscan Jul 20 '12 at 3:50
Ha, I noticed that at first and then totally forgot by the time I got done :) yeah that would be a killer, since the wildcard match would have to be run against every input element on the DOM for each iteration of the loop. The other stuff can't hurt either. – jamietre Jul 20 '12 at 3:53
Since selectors are always matched right to left, using find is actually much faster. – Joseph Silber Jul 20 '12 at 16:18
Learn something new every day. I have to say I am astounded that Chrome doesn't optimize even for ID selectors. Apparently they are not always matched this way, though, in your test results Opera's native operation is faster. – jamietre Jul 20 '12 at 16:36
.. revised answer for posterity. – jamietre Jul 20 '12 at 16:53
show 4 more comments

The best that I can see if changing the IFs to switches, but overall, the speed of execution will not change.

switch (val){

case "N":
// Code for N
break;

case "S":
// Code for S
break;

case "D":
// Code for D
break;

default:
// For the rest, use "not" to subtract Y from the equasion.
}
share|improve this answer
Op's issues are likely DOM traversal related (especially as IE8 is very slow at that), so, I kinda doubt this helps. – JayC Jul 20 '12 at 3:37
I said that before I suggested any sort of code... – TurdPile Jul 20 '12 at 3:42

I think you should use behind C# code in aspx.cs to do this. If the visibility and style of the elements(buttons, table cells) is NOT interactive.
This solution doesn't need javascript at all, and will get better performence in browsers.

share|improve this answer
ASP.NET rowdatabound is too slow. I dont think that will help because i am trying to improve my application performance – Tuscan Jul 20 '12 at 3:52
ASP.NET rowdatabound run on the server, JS run in the browser. Your question is to optimize JS code, this can only affect the performance in browser, not the server side. If you use ASP.NET rowdatabound, you use the server to do the job, and leave Browser free. So your program's performance in browser is got optimized. – Jerry Jul 20 '12 at 3:54
But, If we talk about overall performance which process will be faster? – Tuscan Jul 20 '12 at 3:58
Server's are generally much faster than a personal computer. – TurdPile Jul 20 '12 at 4:00
1  
I haven't used webforms in a while but I remember gridview/listview being a huge pain to manipulate the individual row data in any way other than simple templating actions from the server. His demands here aren't excessive, this seems reasonable to do on the client. Templated grid controls that run entirely on the client are common these days and they actually perform a lot better than server models because of low latency & minimized data transfer, the only reason he had a performance problem was due to an egregious error with a selector. – jamietre Jul 20 '12 at 4:00
show 3 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.