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?

Improve this question
5
  • 1
    So you want the 2nd, 3rd and 4th inserts to fail? Commented Feb 8, 2013 at 2:44
  • Yes, because the account should already exist(but doesn't just yet). Commented Feb 8, 2013 at 2:47
  • 3
    Best practice is to add a unique index on email and then handle the insert error if there's a dupe as another flavor of the "Account already exists." error. Commented Feb 8, 2013 at 2:56
  • That sounds like a good way to handle it. Commented Feb 8, 2013 at 3:21
  • You can also try to approach problem with promises, see: stackoverflow.com/questions/11912573/… Commented Feb 8, 2013 at 9:11

3 Answers 3

Reset to default
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
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

One other option worth mentioning is 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.
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).
0

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

Improve this answer

Comments

-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();
            }

        }
    });
}
Improve this answer

3 Comments

I understand why its happening, what I'm wanting to know is what the best way to work around it is.
I added the code above, to show whats the right way of doing it in JavaScript. Or, you can use the Step library github.com/creationix/step
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 book.mixu.net/node/ch7.html

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.