-2

I have the following for loop which works correctly in my code. However it needs to do multiple api calls as it loops through, which is taking a long time, so i am trying to figure out whether i could do a promise all and run the api calls all at once.

But really not sure how to go about it, with how my code works, so hoping someone might be able to help.

Below is my code:

let terms = []
this.selectedGroup.groups = ['A', 'B', 'C', 'D', 'E', 'F']

//loop through each term
for (let term of this.terms) {
  let subjects: any = [];

  //loop through each terms assessments
  for (let assessment of term.assessments) {

    //loop through the assessment year groups
    for (let assessmentGroup of assessment.year_groups) {

      //only get the assessments for the particular year group we are looking at
      if (this.selectedGroup.groups.includes(assessmentGroup.name)) {

        //get the subject data for this assessment
        let newSubjects = await this.dataService.getData(
          assessment,
          assessmentGroup,
          this.selectedGroup.students
        );

        //add the assessment subjects to the overall subjects array
        subjects = subjects.concat(newSubjects)
      }
    }
  }

  //push all the subjects for the term into a terms array
  terms.push({
    name: term._id,
    subjects: subjects,
  });
}

Is there anyway of introducing a promise all to this, so the api calls can run at once? Thanks in advance!

  • Does this answer your question? [How to use Promise.all() after Promises created in for loop](https://stackoverflow.com/questions/42180299/how-to-use-promise-all-after-promises-created-in-for-loop) – Rickard Elimää Dec 17 '22 at 09:52

2 Answers2

1

Here are the steps to take:

  1. Remove the await keyword where you currently have it. So now subjects will be an array of promises.

  2. Add await Promise.all in your push call:

    terms.push({
        name: term._id,
        subjects: (await Promise.all(subjects)).flat(),
    });
    

That's it: now there will multiple pending promises until one item is pushed to the terms array. This should already give some improvement.

If you want to have even more simultaneous pending promises, then also make terms an array of promises:

terms.push(Promise.all(subjects).then(subjects => ({
    name: term._id,
    subjects: subjects.flat(),
})));

And then, after the outermost loop, have a one-for-all await:

terms = await Promise.all(terms);

Demo

I created some mock data along the structure that your code expects, with a mock getData function, ...etc, so that code runs without errors. Here are the three versions of the code:

Original code

const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
const app = {
    selectedGroup: {groups: ["A"],students: ["Student1", "Student2"]},
    terms: [{assessments: [{year_groups: [{name: "A"}]}, {year_groups: [{name: "E"}]}],_id: 123}, 
            {assessments: [{year_groups: [{name: "C"}]}],_id: 567}],
    dataService: {
        async getData(assessment, group, students) {
            await delay(100);
            return students.map(student => "++" + student);
        }
    },
    
    async f() { // The function in the question (no change)
        let terms = [];
        this.selectedGroup.groups = ['A', 'B', 'C', 'D', 'E', 'F'];
        for (let term of this.terms) {
          let subjects = [];

          //loop through each terms assessments
          for (let assessment of term.assessments) {

            //loop through the assessment year groups
            for (let assessmentGroup of assessment.year_groups) {

              //only get the assessments for the particular year group we are looking at
              if (this.selectedGroup.groups.includes(assessmentGroup.name)) {

                //get the subject data for this assessment
                let newSubjects =  await this.dataService.getData(
                  assessment,
                  assessmentGroup,
                  this.selectedGroup.students
                );

                //add the assessment subjects to the overall subjects array
                subjects = subjects.concat(newSubjects)
              }
            }
          }

          //push all the subjects for the term into a terms array
          terms.push({
            name: term._id,
            subjects: subjects,
          });
        }
        return terms;
    }
};

app.f().then(console.log);

With subjects being promises

const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
const app = {
    selectedGroup: {groups: ["A"],students: ["Student1", "Student2"]},
    terms: [{assessments: [{year_groups: [{name: "A"}]}, {year_groups: [{name: "E"}]}],_id: 123}, 
            {assessments: [{year_groups: [{name: "C"}]}],_id: 567}],
    dataService: {
        async getData(assessment, group, students) {
            await delay(100);
            return students.map(student => "++" + student);
        }
    },
    
    async f() { // The function with first modification
        let terms = [];
        this.selectedGroup.groups = ['A', 'B', 'C', 'D', 'E', 'F'];
        for (let term of this.terms) {
          let subjects = [];

          //loop through each terms assessments
          for (let assessment of term.assessments) {

            //loop through the assessment year groups
            for (let assessmentGroup of assessment.year_groups) {

              //only get the assessments for the particular year group we are looking at
              if (this.selectedGroup.groups.includes(assessmentGroup.name)) {

                // Removed `await`
                let newSubjects = this.dataService.getData(
                  assessment,
                  assessmentGroup,
                  this.selectedGroup.students
                );

                //add the assessment subjects to the overall subjects array
                subjects = subjects.concat(newSubjects)
              }
            }
          }

          //Wait for all subjects promises to resolve
          terms.push({
            name: term._id,
            subjects: (await Promise.all(subjects)).flat(),
          });
        }
        return terms;
    }
};

app.f().then(console.log);

With subjects and terms being promises

const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
const app = {
    selectedGroup: {groups: ["A"],students: ["Student1", "Student2"]},
    terms: [{assessments: [{year_groups: [{name: "A"}]}, {year_groups: [{name: "E"}]}],_id: 123}, 
            {assessments: [{year_groups: [{name: "C"}]}],_id: 567}],
    dataService: {
        async getData(assessment, group, students) {
            await delay(100);
            return students.map(student => "++" + student);
        }
    },
    
    async f() { // The function with second modification
        let terms = [];
        this.selectedGroup.groups = ['A', 'B', 'C', 'D', 'E', 'F'];
        for (let term of this.terms) {
          let subjects = [];

          //loop through each terms assessments
          for (let assessment of term.assessments) {

            //loop through the assessment year groups
            for (let assessmentGroup of assessment.year_groups) {

              //only get the assessments for the particular year group we are looking at
              if (this.selectedGroup.groups.includes(assessmentGroup.name)) {

                // Removed `await`
                let newSubjects = this.dataService.getData(
                  assessment,
                  assessmentGroup,
                  this.selectedGroup.students
                );

                //add the assessment subjects to the overall subjects array
                subjects = subjects.concat(newSubjects)
              }
            }
          }

          //push a promise for all subjects promises to resolve
          terms.push(Promise.all(subjects).then(subjects => ({
            name: term._id,
            subjects: subjects.flat(),
          })));
        }
        // One final spot to await all terms promises
        terms = await Promise.all(terms);
        return terms;
    }
};

app.f().then(console.log);

As you can see the output is the same for each version.

But all of them output duplicate data: I don't know what your getData does, but if it uses the students argument, like I did, for building the result, then surely the result will have duplicate students. Similar duplication can occur because of the assessment parameter, which also is repeated by the inner loop. As you said your code was correct, I leave that for you to further assess.

trincot
  • 317,000
  • 35
  • 244
  • 286
  • Thank you! That is working and much faster! –  Dec 16 '22 at 20:10
  • 1
    I added `flat` calls in a later edit, because I had a doubt that maybe `getData()` resolves to an **array**. That I could not be sure about looking at your code... But it doesn't hurt to do that `flat()` anyway, even when not needed. – trincot Dec 16 '22 at 20:11
  • The overwriting is what you intended: that is what `concat` does: it creates a longer array from the original array with the additional promise added to it. But if your `newSubjects` end up to **resolve** to identical arrays, then you have a bug in `getData`. You must make sure it returns a promise that will resolve to a new array each time, not a mutation of the previous one. – trincot Dec 17 '22 at 09:08
  • Looking a bit more at your code, you have `this.selectedGroup.students` being passed to `getData` with each call of it -- there is no reason to believe that this `this.selectedGroup.students` would be different for each call. But this really goes beyond your question, as that would also be a problem in your original code, yet you claim it works correctly. I now doubt that. – trincot Dec 17 '22 at 09:30
  • I added demo code, which outputs the same results for all three versions of the code: your original code, and my two suggested versions. Of course, I don't know what logic `getData` uses, but it might be you have some problems in that code. – trincot Dec 17 '22 at 09:46
0

You can put the this.dataService.getData call in a array and in the of the assessment.year_groups loop resolve all promises with Promises.all method.

let terms = []
let promises = [];
this.selectedGroup.groups = ['A', 'B', 'C', 'D', 'E', 'F']

//loop through each term
for (let term of this.terms) {
  let subjects: any = [];

  //loop through each terms assessments
  for (let assessment of term.assessments) {

    //loop through the assessment year groups
    for (let assessmentGroup of assessment.year_groups) {

      //only get the assessments for the particular year group we are looking at
      if (this.selectedGroup.groups.includes(assessmentGroup.name)) {

        //get the subject data for this assessment
         promises.push(this.dataService.getData(
          assessment,
          assessmentGroup,
          this.selectedGroup.students
        ));
      }
    }
   Promise.all(promises).then(v => subjetcs.concat(v)) # resolve the promise here
  }

  //push all the subjects for the term into a terms array
  terms.push({
    name: term._id,
    subjects: subjects,
  });

Lohanna Sarah
  • 56
  • 1
  • 3