1

I have few api call requests which i am trying to create promises and then execute all using Promise.all but, its giving empty value instead of array. Below is my code.

function getUser(arrUser) {
  var student = [];
  return new Promise((resolve, reject) => {
    if (arrUser.length > 0) {
      var promises = arrUseridRequest.map(userRequeset => {
        return getRequest(userRequeset).then(result => {
          student.push(JSON.parse(result.body).result[0].Name);
          console.log(student); //This is giving right details.
        }).catch(error => {
          reject(error);
        });
      });

      Promise.all(promises).then(StuName => {
        resolve(StuName.join());
      })
    }
  });
}

and this is how i am trying to get the values at once:

getUser('123').then(student => {
  console.log(Student) //Coming as empty
});

getRequest is my api call nodeJs request module. What's wrong in my code?

Lara
  • 2,821
  • 7
  • 39
  • 72
  • 2
    You are using promises as a fancy callback system. Promise represents a value itself. So you need to create an array of promises returning something, not populating some external array. Then use Promise.all to produce final result. – Yury Tarabanko Jul 15 '20 at 17:20
  • `console.log(Student)` Is this a typo? Shouldn’t this be `student` with small `s`? – Abhishek Sharma Jul 15 '20 at 17:20
  • not sure you need to the student array to store the results or even the `new Promise` bit. I think... nm @YuryTarabanko just replied what I was going to get to. – akaphenom Jul 15 '20 at 17:20
  • 2
    Where is `arrUser` parameter used? What is `arrUseridRequest`? – adiga Jul 15 '20 at 17:20
  • Also you rarely need to use `Promise` constructor directly. Definitely not in the example you have. `Promise.all` creates one for you. – Yury Tarabanko Jul 15 '20 at 17:21
  • many issues with the code sample. My recommendation is to limit what you are trying to do and functionality from a basis of it working... e.g. get it working for jsut one record, not an array. – akaphenom Jul 15 '20 at 17:21
  • @YuryTarabanko Could you please help me how that can be done? – Lara Jul 15 '20 at 17:23
  • 2
    `function getUsers(users) { return Promise.all( users.map(user => /* create a promise returning what you need for each user*/)}` can't write anything more useful because you have some random variables popping here and there in your code snippet. – Yury Tarabanko Jul 15 '20 at 17:31
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Jul 15 '20 at 17:39
  • `return getRequest(userRequeset).then(result => {` <--- this then returns nothing, you just push to an array. – epascarello Jul 15 '20 at 17:45
  • @epascarello It still returns a promise from the `map` callback – Bergi Jul 15 '20 at 17:49

1 Answers1

2

All your promises fulfill with the value undefined since you're just logging the student names, but not returning them from the then callback. As you seem to be doing only a single request, the array will be [undefined], which is joined into the empty string.

Also avoid the Promise constructor antipattern:

function getUsers(arrUser) {
  const promises = arrUser.map(userId => {
    return getRequest(userId).then(result => {
      const students = JSON.parse(result.body).result;
      return students[0].Name;
    });
  });
  return Promise.all(promises);
}
getUsers(['123']).then(studentNames => {
  console.log(studentNames);
  console.log(studentNames.join());
}).catch(console.error);
Bergi
  • 630,263
  • 148
  • 957
  • 1,375