2

i have a problem with this code:

    var par = [];
    $('a[name]').each(function() {
        if (($(this).attr('name')).indexOf("searchword") == -1) {
            par.push($(this).attr('name'));
            $('.content').empty();
            for (var i = 0; i < par.length; i++) {
                $(".content").append('<a id="par" href="#' + par[i] + '">' + par[i] + '</a><br />');
            }
        }
    });

It causes ie and firefox to popup the warning window "Stop running this script". But it happens only when there is a very very large amount of data on page. Any ideas how to fix it?

2 Answers 2

4

Your code should look like this:

var par = [];
$('a[name]').each(function() {
    if (($(this).attr('name')).indexOf("searchword") == -1) {
        par.push($(this).attr('name'));
    }
});
$('.content').empty();
for (var i = 0; i < par.length; i++) {
    $(".content").append('<a id="par" href="#' + par[i] + '">' + par[i] + '</a><br />');
}

There is no reason for the second loop to be inside the first - that will just cause a lot of unneeded work.

You can make this code a bit simpler by removing the par array and the second loop, and just creating the content inside the first loop:

$('.content').empty();
$('a[name]').each(function() {
    var name = $(this).attr('name');
    if (name.indexOf("searchword") == -1) {
        $(".content").append('<a id="par" href="#' + name + '">' + name + '</a><br />');
    }
});
3
  • It is not only unneeded, this is the problem. Apr 26, 2010 at 8:20
  • +1 for the remark about removing the array altogether, and just doing the append in the first iteration. provided that the array is not used anywhere else, that's obviously the way to go Apr 26, 2010 at 8:21
  • deleting my own answer as it is redundant now. all that i want to keep from it can be reduced to a comment. the selector could be simplified to $('a[name]:not(a[name*=searchword])') and the if-statement could be dropped. a matter of preference, perhaps, but i'd say that's neater... Apr 26, 2010 at 8:39
0

Browsers run all javascript (and most page interaction) on a single thread. When you run a long loop like this with no interruptions, the UI is totally frozen. You should try to make your algorithm have to do less, but in case that's not possible you can use this trick where you do a bit of work, then pause and give the browser control of the UI thread for a bit, then do more work.

var $targets = $('a[name]');
var current = 0;
var i = 0;
function doSomeWork() {
    if (i == $targets.length) return;

    var $t = $targets[i];
    if (($t.attr('name')).indexOf("searchword") == -1) {
        par.push($t.attr('name'));
        $('.content').empty();
        for (var i = 0; i < par.length; i++) {
            $(".content").append('<a id="par" href="#' + par[i] + '">' + par[i] + '</a><br />');
        }
    }
    i++;
    window.setTimeout(arguments.callee, 0);
}

This does one iteration of your loop in a function before yielding. It might be a good idea to do more than just one in a function call, but you can experiment with that. An article on this idea: http://www.julienlecomte.net/blog/2007/10/28/

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.