Function returning map before foreach ended

Question

I have this little program that calculates totals by multiplying a rate and some hours.

The problem I am having is that the function getTasks() always return an empty map. When I log the fields being entered in the map, they are not empty but they are entered after the function returns the map. So I am a bit confused why this is happening.

function getTask(taskId) {
  return rp({
    uri: `${url}/tasks/${taskId}`,
    auth: {
      user: 'user',
      password,
      sendImmediately: false
    },
    json: true
  }).then((task) => {
    return task;
  });
}

function getTasks(entries) {
  const taskIds = [];

  entries.forEach((entry) => {
    const taskId = entry.day_entry.task_id;
    if (!taskIds.includes(taskId)) {
      taskIds.push(taskId);
    }
  });

  const taskRateMap = new Map();

  taskIds.forEach((taskId) => {
    return Promise.resolve(getTask(taskId)).then((res) => {
      if (res.task.id === taskId) {
        taskRateMap.set(taskId, res.task.default_hourly_rate);
        console.log(taskRateMap);
      }
    });
  });
  return taskRateMap;
}

function calculateTasksTotals(id) {
  return co(function* calculateTotalsGen() {
    let total = 0;

    const entries = yield getEntriesForProject(id);

    const tasks = getTasks(entries);

    entries.forEach((entry) => {
      const rate = tasks.get(entry.day_entry.task_id);
      total += rate * entry.day_entry.hours;
    });
    return total;
  });
}

calculateTasksTotals(id)

score 0 · Answer 1 · answered May 24 '17 at 17:35

You have an issue with your Promises. Try using Promise.all() providing all the promises as input, and return your map only when all the promises have been resolved. Otherwise directly return your Promise.all() and create the map in the calling method.

So something like:

const tasksPromises = [];
  taskIds.forEach((taskId) => {
    tasksPromises.push(getTask(taskId)); 
  });
return Promise.all(tasksPromises);

Then inside your calling method resolve the promises through then and you will have as parameter of the callback function an array where each element is the returned value of the corresponding promise.

Alex · Answer 2 · 2017-06-20T20:27:51.317

I believe this happening because the taskRateMap isn't being populated before it's being returned. You might want to look into Promise.all() and consider wrapping

promises = taskIds.map((taskId) => {
return Promise.resolve(getTask(taskId)).then((res) => {
  if (res.task.id === taskId) {
    return [taskId, res.task.default_hourly_rate];
    console.log(taskRateMap);
  }
});

return Promise.all(promises).then(v => /* taskRateMap*/)

score 0 · Accepted Answer · answered May 24 '17 at 20:10

There are multiple problems with your code:

First off, as soon as you have any asynchronous operation involved in a function, the result of that function is going to be asynchronous. You simply cannot return it synchronously. The async operation finishes sometime later. The function itself returns BEFORE the async operation finishes.
So, you return a promise from any function that uses an async operation and the caller uses that promise to know when things are done or to get the final result.
Your function getTask() is fine. It returns a promise. The .then() inside that function is redundant and not needed since task appears to already be the resolved value of the promise.
Your function getTasks() is trying to synchronously return taskRateMap, but as you've seen in testing, none of the promises have resolved yet so there are no values in taskRateMap yet. In my code version, I use Promise.all() internally to know when all the getTask() operations are done and I return a promise who's resolved value is the taskRateMap object.
The caller of getTasks() can then use that promise and a .then() handler to get access to the taskRateMap object.

Here's one way to implement getTasks():

function getTasks(entries) {
    // get all unique task_id values from the entries array
    const taskIds = Array.from(new Set(entries.map(entry => entry.day_entry.task_id)));
    const taskRateMap = new Map();

    // use Promise.all() to know when the whole array of promises is done
    // use tasksIds.map() to build an array of promises
    return Promise.all(taskIds.map(taskId => {
        // make this promise be the return value inside the .map() callback
        // so we will end up with an array of promises that will be passed to
        // Promise.all()
        return getTask(taskId).then(res => {
            if (res.task.id === taskId) {
                taskRateMap.set(taskId, res.task.default_hourly_rate);
            }
        })
    })).then(() => {
        // make final resolved value be the taskRateMap
        return taskRateMap;
    });
}

getTasks(entries).then(taskRateMap => {
    // use taskRateMap here
});

Function returning map before foreach ended

3 Answers3