Return of promise inside forEach

Question

function getNamesById(nameIds) {
    var defer = $q.defer();

    var result = [];
    nameIds.forEach(function (nameId) {
        account.getNameById(nameId).then(function (name) {
            result[nameId] = name;
        });
    });
    defer.resolve(result);
    return defer.promise;
}

I have the above code witch is obviously not working as expected. I have to iterate an array of id's and construct another array having the id's as keys, and the values taken from an async function

You [cannot use `forEach` with promises](https://stackoverflow.com/q/37576685/1048572). Also, avoid the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, May 24 '17 at 11:34
@JaromandaX `forEach` + `push` is so ugly that I don't want to consider it :-) — Bergi, May 24 '17 at 12:36

Jamiec · Accepted Answer · 2017-05-24T18:36:20.240

You should just return another promise by using $q.all

function getNamesById(nameIds) {

    var result = [];
    nameIds.forEach(function (nameId) {
        result.push(account.getNameById(nameId));
    });
    return $q.all(result);
}

and then chain that on elsewhere

getNamesById(someNameIds)
     .then(...);

A clearer way to write this is using .map instead of declaring an array and using forEach to push into it:

function getNamesById(nameIds) {
    return $q.all(nameIds.map(function (nameId) {
        return account.getNameById(nameId)
    }));
}

If using ES6 (and your method definitely ignores all but the first argument) this can be simplified further:

let getNamesById = nameIds => Promise.all(nameIds.map(account.getNameById));

So from comments, I surmise that you're trying to get an associative array back with nameId as the key and the result of the async call as the value. There is one more step you require - You need to return a value from your then which you can consume later

function getNamesById(nameIds) {
    return $q.all(nameIds.map(function (nameId) {
        return account.getNameById(nameId)
                    .then(function(name){
                        return {id:nameId, name:name};
                     })
    }));
}

When you consume this method, you can easily reduce the result to your associative array

getNamesById(["a","b","c"])
    .then(function(results){
        var values = results.reduce(function(p,n){
            p[n.id] = n.name;
            return p;
        },{});
        // values is your associative array with keys a,b & c
     });

Written out in ES6 makes this much less verbose - for completeness:

function getNamesById(nameIds) {
    return $q.all(nameIds.map(
               nameId => account.getNameById(nameId)
                    .then(name => ({id:nameId, name:name}))
    );
}

and

getNamesById(["a","b","c"])
    .then(results => {
        var values = results.reduce((p,n) => {
            p[n.id] = n.name;
            return p;
        },{});
        // values is your associative array with keys a,b & c
     });

using `nameIds.map(account.getNameById)` can be problematic if function `account.getNameById` doesn't ignore all but the first argument (I've been stung **once** like that :p (what is a P/A rant?) — Jaromanda X, May 24 '17 at 12:35
@JaromandaX thanks. Answer updated. Your addition to your answer was *very* [passive/agressive](https://en.wikipedia.org/wiki/Passive-aggressive_behavior) and seemed directed at me by the quote `apparently just blurting out the answer first is the accepted way these days`. My answer was far from "blurted out" - but may have been missing a bit of detail which I admit should have been included in the first place. — Jamiec, May 24 '17 at 13:06
wasn't intended that way - so many accepted answers seem to be the quickest rather than the best (in my opinion) - perhaps it's my opinion that is at fault :p — Jaromanda X, May 24 '17 at 13:08
@JaromandaX It's a well known problem here. You're not the first to point it out. — Jamiec, May 24 '17 at 13:08
I used the forEach approach because I needed an associative array with the nameId as the key, and name as value — uromay, May 24 '17 at 16:54
@uromay then you want `.reduce` not `.map` but you need an array of promises to pass to `$q.all` in any case! so somewhere or other you'll have a `.map` — Jamiec, May 24 '17 at 18:23

Return of promise inside forEach

1 Answers1