Node.js callback closure/scope Is it a bad idea to use the notfound callback like this? If so, why not? (bad scope/closure? I'm a noob node.js dev.)
Is it a bad idea to use the saveCallback callback like this? I have a feeling it's dangerous, but I don't want to declare saveCallback before queryCallback as save happens after query.




create: function(registerDetails, found, notfound) {

    var query = db.User
        .findOne({ username: registerDetails.username });

    var queryCallback = function(err, user) {
        if (user != null) found(user);
        else {
            var newUser = new db.User();
            newUser.username = registerDetails.username;
            newUser.password = registerDetails.password;
            newUser.save(saveCallback);
        }
    };

    var saveCallback = function(err, user) {
        notfound(user);
    };

    query.exec(queryCallback);
},


What's a more elegant way of writing this so that

// 1) don't nest like crazy.
.exec(function(){
    if () {
        newUser.save(function(){
            notfound(); // really? because we know node.js will ever chain 2 callbacks max. &lt;/sarcasm&gt;
        }
    }

// 2) don't make people read "down and back up"
var saveCallback = function() {
    // Dejavu... I've been here before...
};
var queryCallback = function() {
    saveCallback(); // Back from last line, ohhhhh this is what saveCallback used for. Go back up.
};
query.exec(queryCallback); // Ohhhhh so that's what it's used for. Now go back up and read what it does.
Interesting question,

I would go with this:

create: function (registerDetails, found, notfound) {

    db.User.findOne({
        username: registerDetails.username
    }).exec(queryCallback);

    function queryCallback(err, user) {
        if (user != null) {
            found(user);
        } else {
            var newUser = new db.User();
            newUser.username = registerDetails.username;
            newUser.password = registerDetails.password;
            newUser.save(saveCallback);
        }
    };

    function saveCallback(err, user) {
        notfound(user);
    };
},


Basically, I 


Used the fact that JavaScript does hoisting, you can use functions prior to having them declared. Given this I put the .exec() on top, I also did away with query which I would have called userQuery.
I put proper braces and newlines around if( user!= null ), to be tested, but if you can get away with it, you should consider if(user)
I moved from var xx = function() to function xx() because anonymous functions in stacktraces are not fun