2

I'm using Mongoose with Node.js and have the following code that will call the callback after all the save() calls has finished. However, I feel that this is a very dirty way of doing it and would like to see the proper way to get this done.

function setup(callback) {

  // Clear the DB and load fixtures
  Account.remove({}, addFixtureData);

  function addFixtureData() {
    // Load the fixtures
    fs.readFile('./fixtures/account.json', 'utf8', function(err, data) {
      if (err) { throw err; }
      var jsonData = JSON.parse(data);
        var count = 0;
        jsonData.forEach(function(json) {
          count++;
          var account = new Account(json);
          account.save(function(err) {
            if (err) { throw err; }
            if (--count == 0 && callback) callback();
          });
        });
    });
  }
}
staackuser2
  • 12,172
  • 4
  • 42
  • 40
  • I use an [`after`](http://stackoverflow.com/questions/6597493/synchronous-database-queries-with-node-js/6620091#6620091) function for this functionality. Simple utility functions like that can help a lot. – Raynos Jul 14 '11 at 03:18
  • That is what developers sometimes call "[spaghetti code](http://en.wikipedia.org/wiki/Spaghetti_code)". [This video tutorial from Yahoo!](http://www.yuiblog.com/blog/2010/12/06/video-yuiconf2010-croucher/) explains how to prevent this. I like to use libraries which helps preventing spaghetti code with for example [async](https://github.com/caolan/async) from Caolan McMahon. There can be found some more at [this node_modules section](https://github.com/joyent/node/wiki/modules#async-flow)). – Alfred Jul 13 '11 at 07:33

5 Answers5

5

You can clean up the code a bit by using a library like async or Step.

Also, I've written a small module that handles loading fixtures for you, so you just do:

var fixtures = require('./mongoose-fixtures');

fixtures.load('./fixtures/account.json', function(err) {
  //Fixtures loaded, you're ready to go
};

Github: https://github.com/powmedia/mongoose-fixtures

It will also load a directory of fixture files, or objects.

evilcelery
  • 15,941
  • 8
  • 42
  • 54
  • If using the fixtures library I'd recommend using the following newer project which is faster and more actively maintained: https://github.com/powmedia/pow-mongodb-fixtures – evilcelery Aug 08 '12 at 09:44
  • Yes .. pow-mongodb-fixtures is faster but mongoose-fixtures will verify that your fixtures comply with mongoose model (e.g. fields match etc). It also can use virtual properties which can be very handy for fixture data. – Tony O'Hagan Jan 05 '13 at 14:15
2

I've recently created simpler abstraction called wait.for to call async functions in sync mode (based on Fibers). It's at an early stage but works. It is at:

https://github.com/luciotato/waitfor

Using wait.for, you can call any standard nodejs async function, as if it were a sync function, without blocking node's event loop. You can code sequentially when you need it.

using wait.for your code will be:

//in a fiber
function setup(callback) {

  // Clear the DB and load fixtures
  wait.for(Account.remove,{});

  // Load the fixtures
  var data = wait.for(fs.readFile,'./fixtures/account.json', 'utf8');

  var jsonData = JSON.parse(data);
  jsonData.forEach(function(json) {
    var account = new Account(json);
    wait.forMethod(account,'save');
  }
  callback();
}
Lucio M. Tato
  • 5,639
  • 2
  • 31
  • 30
2

I did a talk about common asyncronous patterns (serial and parallel) and ways to solve them:

https://github.com/masylum/i-love-async

I hope its useful.

masylum
  • 22,091
  • 3
  • 20
  • 20
0

That's actually the proper way of doing it, more or less. What you're doing there is a parallel loop. You can abstract it into it's own "async parallel foreach" function if you want (and many do), but that's really the only way of doing a parallel loop.

Depending on what you intended, one thing that could be done differently is the error handling. Because you're throwing, if there's a single error, that callback will never get executed (count won't be decremented). So it might be better to do:

account.save(function(err) {
  if (err) return callback(err);
  if (!--count) callback();
});

And handle the error in the callback. It's better node-convention-wise.

I would also change another thing to save you the trouble of incrementing count on every iteration:

var jsonData = JSON.parse(data)
  , count = jsonData.length;

jsonData.forEach(function(json) {
  var account = new Account(json);
  account.save(function(err) {
    if (err) return callback(err);
    if (!--count) callback();
  });
});
chjj
  • 14,322
  • 3
  • 32
  • 24
0

If you are already using underscore.js anywhere in your project, you can leverage the after method. You need to know how many async calls will be out there in advance, but aside from that it's a pretty elegant solution.

Dominic Barnes
  • 28,083
  • 8
  • 65
  • 90