timoxley / functional-javascript-workshop

A functional javascript workshop. No libraries required (i.e. no underscore), just ES5.
2.06k stars 441 forks source link

Exercise 15 solution not fully functional without side effects #196

Closed Kageetai closed 6 years ago

Kageetai commented 6 years ago

I already had the feeling with some of the earlier exercises but with exercise 15 even more, that some of the official solutions are not fully functional and without side effects. The official solution to exercise 15 seems suboptimal with the use of forEach instead of a map for example. So I'd like to submit my solution here, to maybe improve that:

function loadUsers(userIds, load, done) {
  done(userIds.map(id => {
    const user = {};
    load(id, result => user = result);
    return user;
  }));
}

module.exports = loadUsers;
timoxley commented 6 years ago

If this passes verification then it's a bug in the workshop since done is called before all of the values are actually loaded. I.e. a consumer of loadUsers cannot guarantee that all of the user values are actually set to their final values at the time done is called. The user object containers are correct but they're not necessarily containing the right values.

Kageetai commented 6 years ago

Oh, I thought map() only returns the new array, when it's finished? Or do you mean because load() is asynchronous?

timoxley commented 6 years ago

@Kageetai because load is asynchronous.