0

I am still quite new to writing code for Node.js (coming from PHP) and sometimes struggle to understand if async operations are working correctly, especially when there are multiple nested database calls/async operations.

For example, in this piece of code (which uses mongo), will the program only finish when deleted has been set on any required accounts? (see todo 1, 2 and 3):

// array of account emails to be compared against the accounts already in the database (using `email`)
var emailsArray; 

accountsDatabase.find({}, {})
    .then(function (accountsInDB) {
        return q.all(_.map(accountsInDB, function (dbAccount) {
            // compare account's email in db with emails array in memory using .email 
            if ((emailsArray.indexOf(dbAccount.email) > -1) === false) {

                // todo 1. should there be another 'then' here or is 'return' ok in order for it to work asynchronously?

                return accountsInDB.updateById(dbAccount._id, {$set: {deleted: true}}); 
            } else {

                // todo 2. is this return needed?

                return;
            }
        }));
    })
    .then(function () {

        // TODO 3. WILL THE PROGRAM POTENTIALLY REACH HERE BEFORE `deleted` HAS BEEN SET ON THE REQUIRED ACCOUNTS?

        callback(); // all of the above has finished
    })
    .catch(function (err) {
        callback(err); // failed
    });
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
timhc22
  • 7,213
  • 8
  • 48
  • 66
  • 1
    Instead of taking a `callback` parameter, you just should `return` the promise that you produced. – Bergi May 05 '16 at 16:25

1 Answers1

2

should there be another 'then' here or is 'return' ok in order for it to work asynchronously?

return accountsInDB.updateById(dbAccount._id, {$set: {deleted: true}});

You can chain another then here if you want/need, but that's not important. What is important is that you return the promise from the function, so that it can be awaited. If you didn't, the function would still work asynchronous, but out of order - it would just continue immediately after having started the update operation.

is this return needed?

else
   return;

Nope. It just returns undefined, like no return would have done as well. You can omit the whole else branch.

WILL THE PROGRAM POTENTIALLY REACH HERE BEFORE deleted HAS BEEN SET ON THE REQUIRED ACCOUNTS?

callback(); // all of the above has finished

No, it won't. The map produces an array of promises, and q.all makes a promise that waits for all of them (and fulfills with an array of their results). The then will wait for this promise that is returned from its callback before the chain proceeds.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Cool thanks, so it should be ok to leave as it is then without any tweaks? Incidentally, I have now added a second return statement: `return accountsInDB.updateById(dbAccount._id, {$set: {deleted: false}});` – timhc22 May 05 '16 at 16:33
  • I would avoid that `callback` thingy, and if at all, it should be `.then(function(res) { callback(null, res); }, function(err) { callback(err); })` [instead of `.then(…).catch(…)`](http://stackoverflow.com/q/24662289/1048572). But other than that it's ok. It's working, right? – Bergi May 05 '16 at 16:38
  • awesome, yes it seems to be working fine, thanks for the tips and clarification :) – timhc22 May 05 '16 at 16:53