Using promise all in foreach loop

1,132 views
Skip to first unread message

Erick Neverson

unread,
Jul 20, 2015, 5:22:25 PM7/20/15
to q-con...@googlegroups.com
Not sure if this is a typical scenario, but I'm trying to populate and array (and eventually an object containing the arrays) recursively with promise all. Below is the code.  What's happening is, after the first iteration, localEncounter never changes again with teh new values.  Where did I go wrong?  Thanks.

var localEncounters = [];
var localEncounter = {};

Promise.all(ids.map(function(id) {
                    return getEncounter(id, client)
                        .then(function (encounter) {
                            encounterObject = encounter;
                            //set the fields for the return object
                            localEncounter['encounterid'] = encounterObject[f_id];
                            localEncounter['screeningid'] = encounterObject[f_screening_id];
                            localEncounter['assessmentid'] = encounterObject[f_clinical_assessment_id];
                            localEncounter['psychevalid'] = encounterObject[f_psych_eval_id];
 
                            //get screening
                            return getScreening(encounterObject[f_screening_id], client);
                        })
                        .then(function (screening) {
                            //set the fields for the return object
                            localEncounter['screeningbegintime'] = screening[f_begin_time];
 
                            //get assessment
                            return getAssessment(localEncounter['assessmentid'], client);
                        })
                        .then(function (assessment) {
                            //set the fields for the return object
                            localEncounter['assessmentbegintime'] = assessment[f_begin_time];
 
                            //get psycheval
                            return getPsychEval(localEncounter['psychevalid'], client);
                        })
                        .then(function (psychEval) {
                            localEncounter['assessmentbegintime'] = psychEval[f_begin_time];
 
                            localEncounters.push(localEncounter);
                        }
                        , function (reason) {
                            console.log(reason); // display reason why the call failed;
                            reject(reason, 'Something went wrong creating the encounter!');
                        })
 
                })).then(function(results) {
                    resolve(localEncounters);
                })

Wout Mertens

unread,
Jul 20, 2015, 7:36:19 PM7/20/15
to q-con...@googlegroups.com

This code is hard to read.
- avoid "global" state
- a['b'] is easier to read as a.b
- you're using localEncounter across many ids that overwrite the same attributes

Instead, you could e.g. pass your accumulated state with "return promiseFn(…).then(function(res){return {state_and_res});

Wout.


--
You received this message because you are subscribed to the Google Groups "Q Continuum (JavaScript)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to q-continuum...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
--

Wout.
(typed on mobile, excuse terseness)

Dylan Mikus

unread,
Jul 20, 2015, 8:09:19 PM7/20/15
to q-con...@googlegroups.com

Briefly looked over what you’re doing, and noticed two things.

  1. Objects are reference to a place in memory, so when you push localEncounter to localEncounters, you are not pushing a unique clone of localEncounter. If you edit localEncounter in the future, it will edit all instances of it pushed to localEncounters.
  2. Why are you creating localEncounter and localEncounters outside of your map function and promises? You could have the map function return a promise for each local encounter and then have Promise.all( ids.map( ... ) ).then(function (localEncounters) { ...}). The result of Promise.all can be local encounters.

Maybe I’m off with what you are doing, but those two things might be related to your issue.


--
You received this message because you are subscribed to the Google Groups "Q Continuum (JavaScript)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to q-continuum...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
Dylan Mikus
BS in Computer Science from CMU
dbm...@gmail.com
Reply all
Reply to author
Forward
0 new messages