31

I've spent far too many hours searching for similar questions and trying solutions, so I hope someone has a solution.

Basically, I would like to be notified when a function a() has completed. The problem is that the function contains an ajax call and a loop that calls b(), which again contains an ajax call.

UPDATED WITH FIDDLE: http://jsfiddle.net/hsyj7/1/

Like so:

// called by main()
function a() {
  return $.ajax("http://url1").pipe(function(data){
    for (var i = 0; i < 2; i++) {
      console.log('a called');
      b();
    }
  });
}

// called by a()
function b() {
  for (var i = 0; i < 2; i++) {
    $.ajax("http://url2", function(data){
      // do something
      console.log('b called');
    }
  }
}

function main(){
  $.when(a()).done(function(){
    console.log('all completed');
  });
}

What I would like to see then is, possibly with both calls to a() at the top:

a called
b called
b called
a called
b called
b called
all completed

Instead I get

a called
all completed
b called
b called

Or some variant thereof.

I am aware that the above code is missing defer functionality in both the loop and in b(). In some of the variants I have tried, the done() handler in main() is never called.

Any one know how to do this?

jokkedk
  • 2,390
  • 2
  • 17
  • 21
  • The only solution I can think of is some kind of counter which decrements as the AJAX calls return and then fires a `complete` event when it reaches `0`. I'm not posting that as an answer as there's got to be something more elegant :) – Rory McCrossan Dec 19 '12 at 11:39
  • Here's a fiddle of the question: http://jsfiddle.net/pxVAE/ – Stuart Burrows Dec 19 '12 at 11:43
  • I had a similar question, that wasn't quite resolved. - http://stackoverflow.com/questions/12273437/jquery-deferred-object-with-nested-ajax-calls – Aesthete Dec 19 '12 at 11:53
  • Do you also want to execute `b()` in sequence or in parallel? I.e. do you want to wait for `b` to finish before you call it a second time? – Felix Kling Dec 19 '12 at 11:53

4 Answers4

30

Yeah, using Deferred is the way to do that:

function a() {
    var def = $.Deferred();

    $.ajax("http://url1").done(function(data){
        var requests = [];

        for (var i = 0; i < 2; i++) {
             requests.push(b());
        }

        $.when.apply($, requests).then(function() { def.resolve(); });
    });

    return def.promise();
}

// called by a()
function b() {
    var def = $.Deferred(),
        requests = [];

    for (var i = 0; i < 2; i++) {
        requests.push($.ajax("http://url2").done(function(data){
            // do something
            console.log('b called');
        });
    }

    $.when.apply($, requests).then(function() { def.resolve(); });

    return def.promise();
}

function main(){
    $.when(a()).done(function(){
        console.log('all completed');
    });
}

//EDIT: Replaced .pipe with .done.

freakish
  • 54,167
  • 9
  • 132
  • 169
  • I'd +1, but I'm out of votes. Just FMI, what's being achieved by using `pipe()` in `a()` (or in this case is it just synonymous with using `then()`?) – Matt Dec 19 '12 at 11:47
  • 1
    @Matt In this piece of code? Absolutely nothing. I assume it is a snippet from larger piece of code. – freakish Dec 19 '12 at 11:50
  • Thanks @freakish, that works! My confusion was when to use resolve() and promise() in the loops. – jokkedk Dec 19 '12 at 12:27
  • -1 for not handling errors. Also, why do you use the `pipe` method when you don't utilize it? – Bergi Feb 13 '14 at 08:24
  • @Bergi Am I supposed to write all of the code for OP? I've simply shown him how to achieve what he wants. – freakish Feb 13 '14 at 08:48
  • No, but it would help if the bad practise of constructing `$.Deferred`s does not spread :-) – Bergi Feb 13 '14 at 08:51
  • @Bergi I agree ( I'll update the question once I have enough time to recall what was it even about :D ). However since it does answer the question I don't think it deserves -1. – freakish Feb 13 '14 at 08:54
  • I think (high-voted) bad-practise answers are a hazard (to the >1k viewers), so I pressed "*answer is not useful*". I'm looking forward to see it corrected! – Bergi Feb 13 '14 at 09:04
  • @Bergi I've fixed it. It should be ok now. – freakish Feb 13 '14 at 09:12
  • Usage of `pipe` was not the problem - it would've been the solution actually. The problem is manually constructing and resolving deferreds. You might want to have a look a my answer below. – Bergi Feb 13 '14 at 09:25
  • @Bergi Then we completely disagree. You've moved a big part of function `a()` outside of it. What if it is called somewhere else directly? I can't agree to such refactoring. We should not modify the original code more then necessary. – freakish Feb 13 '14 at 09:32
  • Sure - check my update. This triviality of the change is the reason to use `then` for composing asynchronous actions. – Bergi Feb 13 '14 at 09:35
  • @Bergi I see, it seems to be fine now (there are still many things for me to learn about jQuery, indeed). However I disagree that using deferreds directly is a problem. Longer? Yes. Incorrect? No. It's just another solution to the problem. I'll leave the answer as it is. People can use your solution, I don't mind. I'll even upvote it. – freakish Feb 13 '14 at 09:42
  • It's an error-prone design. People forget to reject the deferreds when they would need - just like you have. Using `then` automatically takes care of that. – Bergi Feb 13 '14 at 09:44
  • @Bergi I didn't forget about it - I just didn't care. I agree that it is error-prone but that doesn't mean that it is incorrect. Your solution might be better, but it doesn't mean that mine is wrong. – freakish Feb 13 '14 at 09:46
  • @freakish , with reference to your answer I tried a sample to execute a deferred function in loop. However the result is not working as expected. Refer - https://jsfiddle.net/fewtalks/nvbp26sx/1/. This prints output as {{ processed test1 item 0; processed test1 item 1; processed test item 0; processed test item 1; completed}} . However I expect the output as {{ processed test1 item0 processed test item0; processed test1 item1; processed test item0}}. Why the deferred test function is not executing after test1 is completed. – Darknight Jul 19 '16 at 19:08
  • @fewtalks Post this as a new question (with full code copied there) and we'll have a look at it. It's hard to read from your comment what is the expected output. – freakish Jul 19 '16 at 20:14
  • @freakish http://stackoverflow.com/questions/38468428/jquery-deferred-in-each-loop-with-ajax-callback – Darknight Jul 19 '16 at 20:59
2

You could use an Array which is located in a higher context to push Promise / Deferred objects into. Then you could then use jQuery.when alongside Function.prototype.apply to pass all entries as arguments.

(function() {
    var promises = [ ],
        when = Function.prototype.apply.bind( jQuery.when, null );

    function a() {
         promises.push($.ajax("http://url1").pipe(function(data){
             for (var i = 0; i < 2; i++) {
                 console.log('a called');
                 b();
             }
         }));

         return promises;
    }

    function b() {
        for (var i = 0; i < 2; i++) {
            promises.push($.ajax("http://url2", function(data) {
                // do something
                console.log('b called');
            }));
        }
    }

    function main() {
        promises = [ ];

        when( a() ).done(function(){
            console.log('all completed');
        });
    }
}());
jAndy
  • 231,737
  • 57
  • 305
  • 359
  • -1: Multiple calls to `a` will cause a memory leak, because `promises` is a "global" list. – freakish Dec 19 '12 at 11:59
  • 3
    @freakish: its not a global list. OP didn't say anything about multiple calls. Even if that was the case, it would only push more objects into the *Array* which definitely is not a *memory leak*. He would just need to `re-initialize` the *Array*. -1 for that? Are you joking ? – jAndy Dec 19 '12 at 12:02
  • That's why I wrote *global* in quation marks. Global from `a` and `b` point of view. OP clearly gave example of `a` being called twice. The memory leak is there, because you only push to `promises`. – freakish Dec 19 '12 at 12:04
  • 1
    @freakish: you better google for the definition of a *memory leak* bro, pushing data into an array is just not that. But again, resetting that to an empty *Array* is open for everyone. Again, this is not a downvote. Are you joking.. – jAndy Dec 19 '12 at 12:06
  • 1
    Then why didn't you include resetting in your solution? The code leaks memory when called multiple times. True or false? I don't understand why are you constantly trying to make fun of me. Write your solution properly, then I will upvote. – freakish Dec 19 '12 at 12:09
  • 6
    @freakish: Actually, `a()` is only called once in the OP's example. And since all the functions in jAndy's solution are inside a function expression, none of them can actually called ;) That's more worth to be pointed out :) – Felix Kling Dec 19 '12 at 12:09
  • @FelixKling Oh, you're right. Sorry, I thought that `console.log` is before loop. Fair enough, I'll upvote now. – freakish Dec 19 '12 at 12:12
  • 1
    @freakish: I don't want a flamewar. But you should seriously re-think your behavior. Your wording in this *ECMAcript context* is pretty questionable (quotation or not and about memory leaks) + the fact that its not stated the function is getting called more than once. – jAndy Dec 19 '12 at 12:12
  • @jAndy Re-think my behavior? I'm very patient and polite. I'm not your "bro" and I like to use quation marks ( which is not difficult to understand, seriously ) and I don't make fun of other people. Maybe you should re-think your behavior? Now I've made a mistake and I apologize. – freakish Dec 19 '12 at 12:20
  • @FelixKling True, however we all know, that this is just a snippet and perhaps this is bound to some event. Anyway potentially it can lead to memory leak. – freakish Dec 19 '12 at 12:21
  • 1
    @jAndy I think you could even scope and return `promises` inside both of `a()` and `b()`, to get rid of *context state* altogether – Raffaele Dec 19 '12 at 12:22
  • @freakish: Exactly, it's just a snippet... no need to rumble too much about it... ease up everyone, it's Christmas time :) – Felix Kling Dec 19 '12 at 12:22
  • @freakish: I didn't make any fun of you, I was just shocked that I seriously got a downvote, with that reason. The reason wasn't even correct or true (by definition), so that is what I call a wrong behavior on this site. – jAndy Dec 19 '12 at 12:23
  • @FelixKling: what do you mean with *none of them can actually called* ? Code works as expected. – jAndy Dec 19 '12 at 12:28
  • Where are you calling `main()`? – Felix Kling Dec 19 '12 at 12:30
  • @FelixKling: where does the OP call it ? – jAndy Dec 19 '12 at 12:32
  • Fair enough, I just wanted to point out that none of the functions `a`, `b` and `main` are accessible from outside the whole code block. So assuming OP calls `main` *somewhere*, it could not be found with your code. – Felix Kling Dec 19 '12 at 12:33
  • @jAndy Hehe, so maybe we should be very strict and *by definition* ( whatever that means ) answer that the code works correct, because it is not called anywhere. Question closed. Because OP didn't state it. You are so stubborn, while everyone knows that the code you've shown us potentially leads to memory leak. Also it can be easily fixed, so I don't understand this entire attack. And stop offending me all the time. Not correct? Not true? B***ch, please. – freakish Dec 19 '12 at 12:34
  • 1
    @freakish: I truly believe that I'm pretty open minded and not stubborn. But you just "assume" things and based on that, predict *memory leaks* (which still are no memory leaks, still, by definition) and dangerously fool around with words like *global* which are generally used in a whole different way. That plus of course the downvote for this questionable overall behavior. What did you expect me to do ? Say thank you ? – jAndy Dec 19 '12 at 12:38
  • @jAndy Is it that difficult to assume it? Is it so unlikely to happen? You did assume that `main` is called somewhere, right ( otherwise the code works **by definition** )? So I assumed that it is possibly called multiple times. How is it different? And do you have to be so stubborn about "global" word, you grammar freak? Not to mention that we should also teach good practices and what you've shown us is not even close. You don't have to say anything, just fix your solution. OK, that's it for me. I won't argue anymore, as Felix stated is is Christmas time after all. Merry Christmas! – freakish Dec 19 '12 at 12:42
  • @freakish: its not difficult, but quite leaning far out of the window. It's one thing to expect that a code gets executed once (otherwise there is no error or questions) but a different story to predict or assume unknown application logic. And you call me stubborn ;) Beeing picky on your *global* expression is still correct imho. It wouldn't on any other tag, but this question is tagged with *javascript*. Merry xmas. – jAndy Dec 19 '12 at 12:47
  • @jAndy OMG, fair enough, I apologize for misusing "global" word. :) I won't apologize for calling you stubborn, though. :) Merry Christmas! – freakish Dec 19 '12 at 12:50
  • -1. The `promises.push` in `b()` is too late, `$.when()` will not account for them. – Bergi Feb 13 '14 at 08:31
2

The question might be old, but since there's no correct solution yet I'll put an answer here. It properly chains the promises by using .then (previsouly been .pipe) to achieve the requested result:

function a() {
  return $.ajax("http://url1").done(function(data){
    console.log('a called');
  }).then(function(){
    return $.when(b(), b()); // no loop for simplicity
  });
}
function b() {
  return $.ajax("http://url2").done(function(data){
    console.log('b called');
  });
}

function main(){
  a().done(function(){
    console.log('all completed');
  }, function() {
    console.log('an error occured!');
  });
}

Depending on which result data should be available where the nesting/structure might be changed, but the overall ordering is correct.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
1

I believe this can be fixed with callbacks, but a fiddle would have really helped me check for you.

 // called by main()
 function a(callback) {
   //set this to the number of loops that is going to happen
   var number = 2;
   return $.ajax("http://url1", function(data){
     console.log('a called');
     for (var i = 0; i < number ; i++) {
       b();
       if(number===i){
           callback();
       }
     }
   }
 }

 function main(){
    a(function(){
       //Function or code you want to run on completion.. 
    });
 }

Forgive me if this doesn't work, but i think its the right direction.

Jamie Hutber
  • 26,790
  • 46
  • 179
  • 291