Have Geek Brain

Can you help improve this Javascript code?

EDIT2:
I missed the line that updates the UI "currentNode.set_checked (checked)" and it works incredibly fast. Apparently this is the reason for this performance bottleneck, what do you suggest?

Hi SO,

Long story short, I am dealing with Telerik RadTreeView and I want to enable checking of all child nodes if the user has checked the parent node. Simple enough! OK, here's my code that handles the OnClientNodeChecked TreeView event:

function UpdateAllChildren(nodes, checked) {

            var i;
            for (i = 0; i < nodes.get_count(); i++) {
                var currentNode = nodes.getNode(i);
                currentNode.set_checked(checked);                    
                if (currentNode.get_nodes().get_count() > 0) {
                   UpdateAllChildren(currentNode.get_nodes(), checked);                   
                }
            }
        }       

        function ClientNodeChecked(sender, eventArgs) {
            var node = eventArgs.get_node();         
            UpdateAllChildren(node.get_nodes(), node.get_checked());
        }

And here is the markup for the TreeView: 

<telerik:RadTreeView ID="RadTreeView1" runat="server" CheckBoxes="True" OnClientNodeChecked="ClientNodeChecked"></telerik:RadTreeView>

There are quite a few nodes in the tree and this is causing my target browser (ehm, this IE7) to actually slow down while it starts up. Additionally, IE7 displays an error message that asks me to stop running scripts as this could cause my computer to become unresponsive (yes, scary enough). So, what do you suggest optimizing this code?

EDIT: Runs pretty fast in firefox and chrome of course. Needles say, eh?

Thanks in advance

+2


a source to share


7 replies


OK, apparently the problem is that calling set_checked () is the most expensive part. I omitted it from the code and the recursive call went pretty quickly. I also changed the ScriptMode property in the script manager to be "release" and that helped get rid of the annoying IE dialog, but the performance is still slow. Case is closed.



0


a source


Surely save your objects for reuse:



var i;
for (i = 0, n=nodes.get_count(); i < n; i++) {
  var currentNode = nodes.getNode(i);
  currentNode.set_checked(checked);
  var curNodes = currentNode.get_nodes();
  if (curNodes.get_count() > 0) {
    UpdateAllChildren(curNodes, checked);                   
  }
}
+3


a source


Avoid repeating calls to objects. Get the value in a variable and reuse the variable. You don't need to check if the node collection was empty before calling the function, as the function already does this (in a loop condition).

function UpdateAllChildren(nodes, checked) {
  var cnt = nodes.get_count();
  for (var i = 0; i < cnt; i++) {
    var currentNode = nodes.getNode(i);
    currentNode.set_checked(checked);
    UpdateAllChildren(currentNode.get_nodes(), checked);                   
  }
}
+2


a source


Profile him in the correct browser to see where he spends most of his time.

Like @mplungjan, object reuse.

It seems unlikely that the call is get_count

expensive, but if it is you, you can flush the check before recursion.

If the library is slow, you can cheat and simulate click events yourself.

0


a source


It's a guess, but I believe it's set_checked

causing ClientNodeChecked

. This is why it has been called many more times than it needs to be.

The easiest way to verify this is to run it in FF with Firebug and use console.log to see exactly what it is called.

0


a source


I also faced the same problem, but was able to solve it. I have not used the set_checked function. Instead, I used jquery to find the underlying checkbox and set the checked attribute. This is how I did it.

function ClientNodeChecked(sender, eventArgs) {
var node = eventArgs._node;
var checked = node._properties._data.checked

if (node.get_nodes()._array.length == 0) {
    UpdateNode(node, checked);
}
else {
    UpdateAllChildren(node.get_nodes(), checked);
}

}

function UpdateAllChildren(nodes, checked) {
var i;
var currentNode;
for (i = 0; i < nodes._array.length; i++) {
    currentNode = nodes._array[i];
    if (checked) {
        //$(eventArgs._node._children._array[1]._element).find("input:checkbox").attr("checked", "true")
        $(currentNode._element).find("input:checkbox").attr("checked", "true");
    }
    else {
        $(currentNode._element).find("input:checkbox").removeAttr("checked");
    }
    var childNodes = currentNode.get_nodes();
    if (childNodes._array.length > 0) {
        UpdateAllChildren(childNodes, checked);
    }
}

}

function UpdateNode(node, checked) {
if (checked) {
    //$(eventArgs._node._children._array[1]._element).find("input:checkbox").attr("checked", "true")
    $(node._element).find("input:checkbox").attr("checked", "true");
}
else {
    $(node._element).find("input:checkbox").removeAttr("checked");
}

}

0


a source


I noticed that set_checked () is the slowest part of this loop. This is much faster for me:

currentNode.get_checkBoxElement().checked = checked;

Note: get_checkBoxElement () is a telerik function

Obviously this bypasses some of Telerik's things, like checking for disconnected nodes and possibly tracking changes. The get_checked () function on the client side will not return checked nodes and on the server side, they will not show up as checked. This worked for me because I only get checked values ​​on the client side.

Also setting ScriptMode = "Release" in the RadScriptManager of the page helped a little, but not as much as it was done above. This made performance with set_checked () acceptable, though:

<telerik:RadScriptManager ID="RadScriptManager1"     
                         ScriptMode="Release"    
                         runat="server">

0


a source






More articles:


All Articles