$.get in a loop: why the function is performed aft

Question

I am a beginner in Javascript and I feel that there is something wrong with me about the $.get jQuery.

Normally, you can assign it to a function that will execute after the data is retrieved correctly.

But if I put my $.get in a loop, the loop continues to execute even if the data is not yet retrieved, and here is my problem.

Here is my code (this is for GreaseMonkey):

var1 = document.getElementsByClassName("some_class");
i = 0;

while (i < var1.length) {
    url = var1[i].getElementsByTagName("some_tag")[0].href;

    $.get(url, function(data) {
        if (data.contains("some_string")) {
            alert(i);
        }
    });
i++;
}

Here, the alert returns var1.length event if it should returns 1 for exemple.

I try to put an alert(i) just after the url declaration and I understood that i++ was done before the function in my $.get.

This is surely a trivial problem, but I can not grasp the logic to not make this happen.

Answer 1

Here's a little demo for you to investigate further.

$("#output").empty();

var startTime = new Date().getTime();

// try experimenting with async = true/false and the delay
// don't set async to false with too big a delay,
// and too high a count,
// or you could hang your browser for a while!
// When async==false, you will see each callback respond in order, followed by "Loop finished".
// When async==true, you could see anything, in any order.
var async = true;
var delay = 1;
var count = 5;

function createClosure(i) {
    // return a function that can 'see' i.
    // and i's remains pinned within this closure
    return function (resp) {
        var duration = new Date().getTime() - startTime;
        $("#output").append("\n" + i + " returned: " + resp + " after " + duration + "ms");
    };
}

for (var i = 0; i < count; i++) {
    // jsfiddle url and params
    var url = "/echo/html/";
    var data = {
        html: "hello " + i,
        delay: delay
    };

    $.ajax(url, {
        type: "post",
        data: data,
        async: async
    }).then(createClosure(i));
}

var duration = new Date().getTime() - startTime;
$("#output").append("\n" + "Loop finished after " + duration + "ms");

Sample async=true output:

Loop finished after 7ms
0 returned: hello 0 after 1114ms
1 returned: hello 1 after 1196ms
2 returned: hello 2 after 1199ms
4 returned: hello 4 after 1223ms
3 returned: hello 3 after 1225ms

Sample async=false output (and the browser hangs for 5558ms!):

0 returned: hello 0 after 1113ms
1 returned: hello 1 after 2224ms
2 returned: hello 2 after 3329ms
3 returned: hello 3 after 4444ms
4 returned: hello 4 after 5558ms
Loop finished after 5558ms

Answer 2

Since you're using jQuery, you can simplify your code quite a bit:

$('.some_class').each( function( i, element ) {
    var url = $(element).find('some_tag')[0].href;
    $.get( url, function( data ) {
        if( data.contains("some_string") ) {
            alert( i );
        }
    });
});

Changes from the original code are:

1. jQuery calls instead of the getElementsBy* functions.
2. jQuery .each() for the loop.
3. Added missing var where needed. (Very important in any version of the code!)

Note that the use of .each() automatically gives you the same effect as the immediately invoked function expression (IIFE) in another answer, but without the extra complication. That's because .each() always uses a callback function, and that creates the closure needed to preserve the i variable (and element too) uniquely for each iteration of the loop.

You can also do this when you have an ordinary while or for loop, and you still don't need the IIFE. Instead, simply call a function in the loop. Written this way, the code would be:

var $elements = $('.some_class');
for( var i = 0;  i < $elements.length;  i++ ) {
    checkElement( i, $elements[i] );
}

function checkElement( i, element ) {
    var url = $(element).find('some_tag')[0].href;
    $.get( url, function( data ) {
        if( data.contains("some_string") ) {
            alert( i );
        }
    });
}

As you can see, the checkElement function is identical to the .each() callback function. In fact, .each() simply runs a similar for loop for you and calls the callback in exactly the same way as this code. Also, the for loop is more readable than the while loop because it puts all the loop variable manipulation in one place. (If you're not familiar with the for loop syntax it may seem less readable at first, but once you get used to it you will probably find that you prefer the for loop.)

In general, when tempted to use an IIFE in the middle of a loop, try breaking that code out into a completely separate function instead. In many cases it leads to more readable code.

Answer 3

Wrap your $.get function thus:

(function(i) {
    $.get(url, function(data) {
        if (data.contains("some_string")) {
            alert(i);
        }
    });
})(i);

The immediately invoked function expression causes the current value of i that's in the outer scope to be bound via the function's parameter i (which then hides the outer variable). If you like, give the function parameter a different name.

Note that this only fixes the problem you actually stated, which is that the loop variable is incremented independently of the callbacks. If you wish to ensure that the AJAX requests run one at a time then there are other solutions, e.g.:

var els = document.getElementsByClassName("some_class");
var i = 0;

(function loop() {
    if (i < els.length) {
        var el = els[i];
        var url = el.getElementsByTagName("some_tag")[0].href;
        $.get(url).done(function(data) {
            if (data.contains("some_string")) {
                alert(i);
            }
            i++;
        }, loop);   // .done(f1, f2) - see below
     }
})();

The .done() call is in the form .done(callback, loop) and the two functions will be called in order. So the i++ line always happens first, and then it arranges for loop to be called pseudo-recursively to process the next element.