Looking for proper pattern in looping and callbacks in nodejs (async)

Question

I have read several posts and articles on this, but I can't quite get it.

I have a piece of code in my Mongoose model that essentially takes care of inviting people to a project. Given some invitees I look to see if they are in the database and if not I go ahead and create them and update the id in the invitee list.

jslint complains about the loop and I'm struggling with the callbacks (and the correct overall pattern when you have a loop that has db saves with callbacks. Obviously what I want to happen is that the loop fully completes, any new users are added to the db, the ids are updates in the original hash (invitees) then the callback happens.

ProjectSchema.methods.invite = function invite(invitees, callback) {
  var User = mongoose.model('User');
  var emails = _.pluck(invitees, 'email');
  // loop through invited hash to see if we already have those users in the db
  User.find({ 'email': { $in: emails}}, function (err, users) {
    for (var invited = 0; invited < invitees.length; invited++) {
      var found = false;
      // logic here to see if they are already users using the users param
      if (found) {
        // they are already in the db so do unrelated things.
      } else {
        // create new user
        var User = mongoose.model('User');
        var newUser = // set up new user here

        newUser.save(somecallback?);
        // update invitees list with id
      }
    }
    callback(err, invitees);
  });
};

Valentin Waeselynck · Accepted Answer · 2014-09-27 18:41:17Z

Several problems here :

You are declaring local variables inside a for loop. This is discouraged because JavaScript has function scope, not block scope (variable hoisting).
You need some way to synchronize the asynchronous changes you make for each user you save to the DB, i.e "wait" for the loop to be completed.

For 1., I suggest you use Array.map, i.e declare a function

function processUser (invited) {
  // essentially the body of your for loop
}

Then call invitees.map(processUser).

The 2. problem is harder: I suggest you use a library that offers asynchrony support such as Q.

To do that, have the processUser function return a Q promise that is resolved when it has completed, then use Q.all for synchronization. i.e something like :

Q.all(invitees.map(processUser))
  .then(function (completions) {callback(null,invitees);},
        function (err) {callback(err,null);});

Wrapping up :

// import Q
var Q = require('q');

// ...

ProjectSchema.methods.invite = function invite(invitees, callback) {

  var User = mongoose.model('User');
  var emails = _.pluck(invitees, 'email');
  // loop through invited hash to see if we already have those users in the db

  function processUser (invited) {
    var deferredCompletion = Q.defer();

    var found = false;
    // logic here to see if they are already users using the users param
    if (found) {
      // they are already in the db so do unrelated things.
      // ...
      deferredCompletion.resolve(true); // resolve with some dummy value
    } else {
      // create new user
      var newUser = // set up new user here

        newUser.save(function (err, user){
          if (err) {
            deferredCompletion.reject(err);
          } else {
            // update invitees list with id
            // ...
            deferredCompletion.resolve(true);
          }
        });
    }
    return deferredCompletion.promise;
  }

  Q.all(invitees.map(processUser))
    .then(function (completions) {
      callback(null,invitees);
    }, function (err) {
      callback(err,null);
    });
};

Alternatively, you can use async.map or async.mapLimit. The latter is useful if you have a lot of users to process and don't want to overload your database.

Collectives™ on Stack Overflow

Looking for proper pattern in looping and callbacks in nodejs (async)

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related