6

I have the following code:

// Retrieve
var MongoClient = require("mongodb").MongoClient;
var accounts = null;
var characters = null;

// Connect to the db
MongoClient.connect("mongodb://localhost:27017/bq", function(err, db) {
   if(err) { return console.dir(err); }

    db.createCollection('accounts', function(err, collection) {
        if(err) { return console.dir(err); }
        else { accounts = collection; }

        createAccount("bob","bob");
        createAccount("bob","bob");
        createAccount("bob","bob");
        createAccount("bob","bob");
    });
});


function createAccount(email, password)
{
    accounts.findOne({"email":email}, function(err, item) {
        if(err) { console.dir(err); }
        else {
            if(item === null) {
                accounts.insert({"email":email, "password":password}, function(err, result) {
                    if(err) { console.dir(err); }
                    else { console.dir("Account " + email + " created."); }
                });
            }
            else {
                console.dir("Account already exists.")
            }

        }
    });
}

When I run the script the first time, I end up with 4 accounts for bob. When I run it the second time, I get 4 messages that the account already exists.

I'm pretty sure I know why this is, and the solution I have come up with is to use some kind queue for processing each read/write of the database in order one at a time. What I am wanting to know, is whether that is the proper way to go about it, and what would the general best practice for this be?

user2037584
  • 63
  • 1
  • 3

3 Answers3

10

Some languages provide a special language construct to deal with this problem. For example, C# has async/await keywords that let you write the code as if you were calling synchronous APIs.

JavaScript does not and you have to chain the createAccount calls with callbacks.

Some people have developed libraries that may help you organize this code. For example async, step, node-promise and Q

You can also use the fibers library, a native library that extends the JavaScript runtime with fibers / coroutines.

And some people have extended the language with constructs that are similar to async/await: streamline.js, IcedCoffeeScript or wind.js. For example, streamline.js (I'm the author so I'm obviously biased) uses _ as a special callback placeholder and lets you write your example as:

var db = MongoClient.connect("mongodb://localhost:27017/bq", _):
var accounts = db.createCollection('accounts', _);
createAccount("bob","bob", _);
createAccount("bob","bob", _);
createAccount("bob","bob", _);
createAccount("bob","bob", _);

function createAccount(email, password, _) {
    var item = accounts.findOne({"email":email}, _);
    if (item === null) {
        accounts.insert({"email":email, "password":password}, _);
        console.log("Account " + email + " created."); }
    } else {
        console.log("Account already exists.")
    }
}

And, last but not least, new language features such as generators and deferred functions are being discussed for future versions of JavaScript (generators are very likely to land in ES6, deferred functions seem to be a bit stalled).

So you have many options:

  • stick to callbacks
  • use a helper library
  • use the fibers runtime extension
  • use a language extension
  • wait for ES6
Bruno Jouhier
  • 1,013
  • 9
  • 11
  • One other option worth mentioning is [tamejs](https://github.com/maxtaco/tamejs/), which is by the same developers as IcedCoffeeScript - actually, it's the original version that works in plain JS. But for some reason it generates twice as much code (although around the same number of functions) as streamline.js, so I'd recommend streamline.js. Also, streamline.js lets you handle errors more naturally with try/catch and has a fibers option that makes it faster. – Matt Browne Aug 24 '14 at 03:03
  • Also worth noting is that streamline.js can be used with CoffeeScript as well (by applying the Streamline.js transform after the CoffeeScript transform; more details in the docs). – Matt Browne Aug 24 '14 at 03:09
0

Add a unique constraint on email and you will not have to check if user exists anymore!

Silviu Burcea
  • 5,103
  • 1
  • 29
  • 43
-1

JavaScript is asynchronous. accounts.findOne returns immediately, so basically all your 4 statements are getting executed together.

What accounts.findOne does is, it says find one {"email":email} and when you find it, run the function that is in the second argument. Then it returns the function and continues to next CreateAccount statement. In the meanwhile when the results are returned from the harddrive (which takes a lot longer than executing these statements), it goes into the function, and since there is no user, it adds one. Makes sense?

UPDATE This is the right way of doing this in JavaScript.

MongoClient.connect("mongodb://localhost:27017/bq", function(err, db) {
   if(err) { return console.dir(err); }

    db.createCollection('accounts', function(err, collection) {
        if(err) { return console.dir(err); }
        else { accounts = collection; }

        createAccount("bob","bob", function() {
            createAccount("bob","bob", function() {
                createAccount("bob","bob", function() {
                    createAccount("bob","bob", function() {
                     });
                });
            });
        });
    });
});


function createAccount(email, password, fn)
{
    accounts.findOne({"email":email}, function(err, item) {
        if(err) { console.dir(err); }
        else {
            if(item === null) {
                accounts.insert({"email":email, "password":password}, function(err, result) {
                    if(err) { console.dir(err); }
                    else { console.dir("Account " + email + " created."); }
                    fn();
                });
            }
            else {
                console.dir("Account already exists.")
                fn();
            }

        }
    });
}
Tarandeep Gill
  • 1,506
  • 18
  • 34
  • I understand why its happening, what I'm wanting to know is what the best way to work around it is. – user2037584 Feb 08 '13 at 03:21
  • I added the code above, to show whats the right way of doing it in JavaScript. Or, you can use the Step library https://github.com/creationix/step – Tarandeep Gill Feb 08 '13 at 11:26
  • 4
    I'm inclined to down vote this answer based on the update which says: "This is the right way of doing this in Javascript". First of all this about async use in Node.js, not Javascript, there is rarely one "right" way to do anything in code. Hopefully any rational developer will recognize that the nested callbacks in this scenario won't scale and are not ideal solution here. I would concur with this quote: "However, more than a couple of levels of nesting would should be a code smell - time to think what you can abstract out into separate, small modules." via http://book.mixu.net/node/ch7.html – Mark Edington Oct 04 '13 at 17:12