javascriptnode.jspromiseco

Function returning map before foreach ended


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)

Solution

  • There are multiple problems with your code:

    1. 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.

    2. 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.

    3. 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.

    4. 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.

    5. 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
    });