-1

There are similar questions like this one here in stackoverflow, but I can't find one that resolves this in particular. I have two forEach loops in Node and one (this one: s.forEach((su) => { cw1.push(su.courseWorkId); }); ) uses the results of the previous one to do its iteration, but it fires asynchronously before the first one finishes so it always tells me that "const s" (the result of the first forEach loop) is empty. Everything else in the code works fine. Any help would be appreciated. 

const fs = require('fs');
const path = require('path');
const {google} = require('googleapis');
const keys = require("./file.json");
const sc = require('./jwt.js');
const scopes = sc.scopes;

const j2 = new google.auth.JWT(keys.client_email,null,keys.private_key,scopes,'me@email.com');

const classroom = google.classroom({version: 'v1', auth:j2});
const cl = classroom.courses;

let cl1 =  ['34809075556', '34800434710'];
let cw1 = [];

function getwi(){
  cl1.forEach((ids) => {                                                   
    cl.courseWork.list({courseId:ids}, (e,r) => {
      const s = r.data.courseWork;
      s.forEach((su) => { cw1.push(su.courseWorkId); });
    });
  });
}

getwi();
Jason Jurotich
  • 441
  • 4
  • 24
  • Which operations in this code are asynchronous? It looks to me that it's more likely that you're trying to use the values in `cw1` before it's done being populated, but you don't show that code. Please show that code. – jfriend00 Aug 02 '19 at 16:16
  • hmm, what happens here is a forEach is applied to the array cl1 to get all the data about each course (which includes various things like coursework, professors, ids, etc.). This is all stored in r.data.courseWork, and then the second forEach should grab just the courseWorkId from r.data.courseWork and put that in the cw1 array. A ton of info is stored in the r.data.courseWork object so I couldn't throw it all here. The problem is that the second forEach tries to get info from r.data.courseWork before the first forEach is done populating it. – Jason Jurotich Aug 02 '19 at 16:59

1 Answers1

1

Modifying a global variable asynchronously is generally a bad idea. Instead wrap the asynchronous task into a promise, then resolve that promise to the value you need.

  const getCourseWork = courseId => new Promise((resolve, reject) => cl.courseWork.list({ courseId, }, (err, result) => err ? reject(err) : resolve(result)));

Then you can get the result as:

  Promise.all( cl1.map(getCourseWork) )
     .then(result => console.log("The result is:", result.flat().map(it => it.id)))
     .catch(error => console.error("An error occured:", error));

That way, all the calls are done in parallel, and are thus faster as doing one after another.

Read on

Jonas Wilms
  • 132,000
  • 20
  • 149
  • 151
  • 1
    Jonas, I had to slightly modify your answer, but it worked in the end. I appreciate that. – Jason Jurotich Aug 02 '19 at 19:12
  • Why wouldn't you use `util.promisify()` instead of manually writing the promisify code yourself? – jfriend00 Aug 02 '19 at 20:53
  • @jfriend00 cause that hides whats actually happening under the hood. By explicitly creating the promise it is easier to understand IMO. – Jonas Wilms Aug 02 '19 at 21:02
  • I understand your teaching point, but you also want to teach someone to use the tools built into node.js rather than rewriting them yourself. Even better to learn that this is what `util.promisify()` does for you. – jfriend00 Aug 02 '19 at 21:14
  • @jfriend00 just tried it, couldn't come up with an elegant util.promisify version (as the context has to be bound and `courseId` has to be wrapped into an object) – Jonas Wilms Aug 02 '19 at 21:18
  • I had to compress this a lot to get it in here, but the first line ended up being like this: const getcw = ids => new Promise((resolve, reject) => { cl.courseWork.list({ courseId:ids }, (e,r) => resolve(r.data.courseWork)) }); – Jason Jurotich Aug 02 '19 at 22:33
  • 1
    Noo! Use the code shown, then do `results.flat()`. Dont push to an areay outside of the promise, thats risky (cause someone migjt access it before its ready) – Jonas Wilms Aug 02 '19 at 22:37
  • 1
    Bingo! ok, that worked. Now I just need to study more. Thanks! Second line was: Promise.all( cl1.map(getcw)).then( r => {r.flat().map(c => c.id); }) – Jason Jurotich Aug 02 '19 at 23:08