3

I have created a for loop that loops the number of times that an element appears in a container. The for loop grabs some data from the HTML and creates a JSON url which will then return a value. That value should then be added to the HTML in the appropriate place.

The problem seems that the for loop completes before all of the Ajax calls are made, so only the last value is being added to the HTML. I thought that I could make sure that the readystate is equal to 4, but that solution did not work. I also tried using complete, rather than success as an Ajax Event. Any insights? Here is my the code.

for(var index = 0; index < $('#wizSteps #step6 label').length; index++){
    var priceCount;
    console.log(index);
    var currentSelect = $('#wizSteps #step6 label[data-pricepos="'+index+'"]');
    url = 'http://www.thesite.com/api/search.json?taxonomy=cat3435' + currentSelect.find('input').attr('name');
    jQuery.ajax({
        url: url,
        dataType: "JSON",
        success: function( data ){
            var totalResult = data.totalNumberOfResults;
            console.log(currentSelect);
            currentSelect.find('.itemCount').text(totalResult);

        }     
    });
}
pizzarob
  • 11,711
  • 6
  • 48
  • 69
  • Ajax is asynchronous. You are starting them in order, but they will complete at a later time, possibly in a different order. – Kevin B Jan 20 '14 at 18:10
  • 4
    Regarding `currentSelect`: [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – Jonathan Lonowski Jan 20 '14 at 18:11
  • have you tried adding `async: false,` to your AJAX options right before `url: url,`? – MonkeyZeus Jan 20 '14 at 18:20
  • @MonkeyZeus I don't know if he has, but I know he shouldn't. – Jason P Jan 20 '14 at 18:20
  • 1
    `async: false` will surely solve the problem, but it will lock the browser up during the request, making your page look broken. **Do not use `async: false` in this case.** – Kevin B Jan 20 '14 at 18:21
  • I give +1 to the question. I think that the question raises a very interesint topic – Igor Chubin Jan 20 '14 at 18:25

4 Answers4

2

That is ok, the calls are not supposed to be done this way. They are only initiated in the loop.

Ajax is asynchronous. The queries are completed later, may be in different order.

If you want to be sure that every call is completed before you do the next one, you must integrate the next call into the callback function of the previous.

In your case the variable may be overwritten in the call back function. You can learn more on this here:

Another interesting question/discussion related to the topic:

It does not directly answer your question, but helps to understand the problem deeper. The point is that you probable don't need the loop at all (or you do but in a completely different form).

Community
  • 1
  • 1
Igor Chubin
  • 61,765
  • 13
  • 122
  • 144
  • How is that ok? The results are incorrect, according to the beginning of the second paragraph in the question. – John Fisher Jan 20 '14 at 18:10
  • It's ok because that's the expected behavior of the given code. Whether or not that's the intended behavior is another story. – Kevin B Jan 20 '14 at 18:10
  • @KevinB: Um.... Isn't that the point of StackOverflow? To get code that doesn't do what is intended to perform the work that *is* intended? – John Fisher Jan 20 '14 at 18:12
  • Sure, but this answer does explain why it is happening, code isn't always required. The updated answer does at least give a suggestion of how to fix it. – Kevin B Jan 20 '14 at 18:14
  • Yeah, that's better. :) – John Fisher Jan 20 '14 at 18:17
2

It looks like you don't necessarily need the requests to finish in order, you just need to keep track of currentSelect in a way that works. For that, you can use the context ajax option:

for (var index = 0; index < $('#wizSteps #step6 label').length; index++) {
    var currentSelect = $('#wizSteps #step6 label[data-pricepos="' + index + '"]');
    url = 'http://www.thesite.com/api/search.json?taxonomy=cat3435' + currentSelect.find('input').attr('name');
    jQuery.ajax({
        url: url,
        dataType: "JSON",
        context: currentSelect,
        success: function (data) {
            var totalResult = data.totalNumberOfResults;
            this.find('.itemCount').text(totalResult);

        }
    });
}
Jason P
  • 26,984
  • 3
  • 31
  • 45
0

You should try creating a recursive function, that you will call again in the success of the ajax call, this way you will be sure that the next ajax call will be called only once the previous call is done.

taxicala
  • 21,408
  • 7
  • 37
  • 66
0

If you want the requests in a sequence, you can work with a queue.

First build the queue:

var queue = [],
    index,
    stepLength = $('#wizSteps #step6 label').length;

for(index = 0; index < length; index++){
    var priceCount;
    console.log(index);
    var currentSelect = $('#wizSteps #step6 label[data-pricepos="'+index+'"]');
    url = 'http://www.thesite.com/api/search.json?taxonomy=cat3435' + currentSelect.find('input').attr('name');
    queue.push([url, currentSelect]);
}

And after that do the serial ajax requests:

function serialAjax() {
    if(queue.length === 0) {
        return;
    }
    var queueData = queue.shift(),
        url = queueData[0],
        currentSelect = queueData[1];

    jQuery.ajax({
        url: url,
        dataType: "JSON",
        success: function( data ){
            var totalResult = data.totalNumberOfResults;
            console.log(currentSelect);
            currentSelect.find('.itemCount').text(totalResult);
            serialAjax();
        }     
    });
};
// call the function
serialAjax();
friedi
  • 4,350
  • 1
  • 13
  • 19