13

I'm trying to use the AngularJS promise/then with a recursive function. But the then-function is not called (none of the error-, success-, notify-callbacks gets called).

Here is my code:

recursive function

loadSection2 = function() {

    var apiURL = "http://..."

    var deferred = $q.defer();

    $http({
        method: "GET",
        url: apiURL
    }).success(function(result, status, headers, config) {
        console.log(result);
        loadCount++;
        if(loadCount < 10) {
            newSectionArray.push(result);
            loadSection2(); 
        } else {
            loadCount = 0;
            deferred.resolve();
            return deferred.promise;
        }
    }).error(function() {
        return deferred.reject();
    });
    deferred.notify();
    return deferred.promise;
};

then

loadSection2().then(function() {
    console.log("NEW SECTIONS LOADED, start adding to document");
    addContent();
}, function() {
    console.log("ERROR CALLBACK");
}, function() {
    console.log("NOTIFY CALLBACK");
}).then(function() {
    loadScrollActive = false;
});

I think, the then has to get the first notify-callback at least. But there is no callback. Is then not working with recursive function?

Manishearth
  • 14,882
  • 8
  • 59
  • 76
Fauphi
  • 360
  • 1
  • 2
  • 11
  • Could you give us a jsfiddle? – Manishearth Dec 16 '13 at 09:27
  • 1
    One thing I see is that you cannot return something from a callback function. So return deferred.promise in .success and .error actually does nothing. Not the cause of the problem though. – Narretz Dec 16 '13 at 09:37
  • 1
    Where is 'loadCount' defined? And, `notify` doesn't work like you think. I have a open issue for that in angular repo -> https://github.com/angular/angular.js/issues/5277 – Rifat Dec 16 '13 at 09:37
  • Hm, there was an answer which solved the problem, but now it disappeared. The problem was, that the "deferred" is initialized at every call of the loadSection2(). @Rifat: notify works for me. The notify-callback gets called as long, as the promise is not resolved. – Fauphi Dec 16 '13 at 09:54

3 Answers3

24

EDIT - 11/11/2015 There is a much cleaner way if you don't care about notify:

loadSection2 = function (){
    var apiURL = "http://..."
    return $http.get(apiURL)
        .then(function(response){
            loadCount++;        
            if (loadCount < 10) {
                newSectionArray.push(response.data);
                return loadSection2();
            }
            loadCount = 0;
        });

};

Old answer available here:

You could continuously pass the promise all the way through.

loadSection2 = function(deferred) {

    if(!deferred){
        deferred = $q.defer();
    }
    var apiURL = "http://..."

    $http({
        method: "GET",
        url: apiURL
    }).success(function(result, status, headers, config) {
        console.log(result);
        loadCount++;
        if(loadCount < 10) {
            newSectionArray.push(result);
            loadSection2(deferred); 
        } else {
            loadCount = 0;
            deferred.resolve();
            return deferred.promise;
        }
    }).error(function() {
        return deferred.reject();
    });
    deferred.notify();
    return deferred.promise;
};
Mathew Berg
  • 28,625
  • 11
  • 69
  • 90
  • 2
    Mathew, I think you forgot to actually pass `deferred` in your recursive call, but this is the right answer. The problem you're having is that you are making a new `deferred` object every time you call the method, instead of using the same one for the whole process. So the first time you call loadSection2, you get deferred1 back, but the deferred that's getting resolved is actually deferred10. Passing the deferred along would help, or you could create the variable outside your method and use closure to access it. – Hylianpuffball Dec 16 '13 at 21:51
  • You are completely right, I meant to have it pass through, answer edited. – Mathew Berg Dec 17 '13 at 00:29
  • @MathewBerg please help me understand the reason of "return deferred promise" from "else" block. I have prepared a q version of this example where in the else block i am not returning deferred.promise; and it works fine. [link](https://gist.github.com/pulakb/94d9d1c96e77537c304e) – Pulak Kanti Bhattacharyya Aug 16 '14 at 06:26
3

I wanted to make a solution that doesn't pass "deferred" variable around and even though I wouldn't say it is a better approach, it works and I learned a from it (jsfiddle).

19/Aug/14 - Updated the code to a much shorter version by removing the creation of another promise in f1(). I hope that it is clear how it relates to the original question. If it isn't let me know in a comment.

f1().then(function() {
    console.log("done");
});

function f1(counter) {

    if (!counter) {
        counter = 0;
    }

    counter++;
    console.log(counter);

    return asyncFunc().then(function() {
        if (counter < 10) {
            return f1(counter);
        } else {
            return;
        }
    });

}

function asyncFunc() {
    var deferred = $q.defer();

    $timeout(function() {
        deferred.resolve();
    }, 100);

    return deferred.promise;
}
VitalyB
  • 12,397
  • 9
  • 72
  • 94
0

Fauphi,

Recursion is totally viable but not a particularly "promisy" approach.

Given that you have deferreds/promises available, you can dynamically build a .then() chain, which delivers a promise of a populated array.

function loadSection2(arr) {
    return $http({
        method: "GET",
        url: "http://..."
    }).then(function(result, status, headers, config) {
        console.log(result);
        arr.push(result);
        return arr;//make the array available to the next call to loadSection2().
    }, function() {
        console.log("GET error");
        return $q.defer().resolve(arr).promise;//allow the chain to continue, despite the error.
        //or I think $q's .then() allows the much simpler form ...
        //return arr; //allow the chain to continue, despite the error.
    });
};

var newSectionPromise = $q.defer().resolve([]).promise;//note that the deferred is resolved with an anonymous new array.
//Now we build a .then() chain, ten long, ...
for (var i=0; i<10; i++) {
    newSectionPromise = newSectionPromise.then(loadSection2);
}
// ... and do something with the populated array when the GETs have done their thing.
newSectionPromise().then(function(arr) {
    console.log(arr.length + " new sections loaded, start adding to document");
    addContent(arr);
}, function() {
    console.log("ERROR CALLBACK");
}).then(function() {
    loadScrollActive = false;
});

untested

What was newSectionArray is now created anonymously and passed down the .then() chain regardless of success/failure of the individual GETs, emerging as arr in the final .then's success handler, where it is passed to addContent(). This avoids the need for member newSectionArray in the outer scope.

Rearranging slightly, loadSection2 could be made anonymous, further reducing the number of members added to the outer scope.

The need for an explicit notification disappears as :

  • there is no longer a master deferred to be notified
  • console.log(result); in the GET success handler provides all the notification necessary.
Beetroot-Beetroot
  • 18,022
  • 3
  • 37
  • 44