Why isn't .then() waiting for the promise?

Question

I am currently working on a angular project, and I am kind of new to it.

I do not understand, why is .then() function not waiting for the promises?

I think it have to do something with that I only have one $q.defer() inside my getAllStats() function? When I try to console.log("testing: ", data); (on the bottom) it only logs out an empty array. Could someone help me please?

This is my code:

function getAllStats(dataArray, nameOfFile) {
    var defer = $q.defer();
    var promises = [];


    for (index in dataArray) {
        if (dataArray[index].indexOf('test') > -1 ) {
            getStats(nameOfFile).then(function (data) {
                 promises.push();
            });
        }
    }

    function last() {
        defer.resolve(promises);
    }

    $q.all(promises).then(last);
    return defer.promise;
};

function getStats(nameOfFile) {
    var defer = $q.defer();
    $http.get(nameOfFile).success(function (data) {
        defer.resolve(data);
    });
        return defer.promise;
};

getAllStats('test.txt').then(function(data) {
        console.log("testing: ", data);
});

You don't need `$q` inside `getStats()` ... `$http` already returns a promise so just return `$http` — charlietfl, Feb 21 '16 at 23:01
Sorry, I forgot to put data inside the push. It should look like this: promises.push(data) ; — Marrips, Feb 22 '16 at 08:26

score 3 · Accepted Answer · edited May 23 '17 at 10:28

3

See the comments in this code:

function getAllStats(dataArray, nameOfFile) {
    var promises = [];
    // using `for (index in dataArray) {` is a bad idea unless
    // dataArray is a non-array object
    for (var index = 0; index < dataArray.length; index++) {
        if (dataArray[index].indexOf('test') > -1 ) {
            // Here is the trick, store the promise itself,
            // don't try to subscribe to it here
            promises.push(getStats(nameOfFile));
        }
    }
    return $q.all(promises);
};

function getStats(nameOfFile) {
    // http.get already returns a promise, see explicit promise creation antipattern
    return $http.get(nameOfFile).then(function(r) { return r.data; });
};

getAllStats('test.txt').then(function(data) {
        console.log("testing: ", data);
});

References:

Deprecation Notice

The $http legacy promise methods success and error have been deprecated. Use the standard then method instead. If $httpProvider.useLegacyPromiseExtensions is set to false then these methods will throw $http/legacy error.

see: $http

edited May 23 '17 at 10:28

Community

1
1

answered Feb 21 '16 at 23:01

Tamas Hegedus

28,755
12
63
97

Just need to be mindful of the fact that each entry in the `data` array will be a response object with `data` property now. – Phil Feb 21 '16 at 23:04
`dataArray.forEach(function(item){})` could make look the code cleaner? – ncubica Feb 21 '16 at 23:07
what about using `$q.when`? – ncubica Feb 21 '16 at 23:09
1

@ncubica I think that's a matter of taste. To be honest I totally agree with you, but I dont want to introduce here functional programming primitives like `forEach`, `map` and `filter`, however those would make the code easy to reason about. – Tamas Hegedus Feb 21 '16 at 23:10
1

I usually only use `$q.when` to wrap function bodies to `return $q.when().then(() => { ... });`, so all accidental errors get converted to rejections, or to transform values to promises from unreliable sources. – Tamas Hegedus Feb 21 '16 at 23:13

jfriend00 · Answer 2 · 2016-02-21T23:25:57.157

Multiple issues wrong here:

You're passing an empty promises array to $q.all(). It has to be an array of promises at the time you pass it to $q.all().
You're creating promises when you can just return the ones you have
getAllStats() expects an array, but you're passing a string.

I'd suggest this overall cleanup of the code that fixes the above issues:

function getAllStats(dataArray) {
    var promises = dataArray.filter(function(item) {
        return item.indexOf('test') !== -1;
    }).map(function(item) {
         return $http.get(item);
    });

    return $q.all(promises);
};

getAllStats(['test.txt']).then(function(data) {
        console.log("testing: ", data);
});

I'd also suggest you read about promise anti-patterns to teach yourself how to use the promises you already have and avoid creating new ones when new ones are not necessary.

P.S. I'm not sure what was the point of the nameOfFile argument since you don't want to be getStats() on the same file over and over again.

score 0 · Answer 3 · answered Feb 21 '16 at 23:00

In your example this block:

for (index in dataArray) {
    if (dataArray[index].indexOf('test') > -1 ) {
        getStats(nameOfFile).then(function (data) {
             promises.push();
        });
    }
}

Takes dataArray, which is a string and runs through it char by char. Also, you are not setting nameOfFile. Change the call to:

getAllStats(['test.txt']).then(function(data) {
    console.log("testing: ", data);
});

And then to make the push to promises be correct do like this:

promises.push(getStats(dataArray[index]));

Why isn't .then() waiting for the promise?

3 Answers3