Node.js callback closure/scope

Question

• 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. </sarcasm>
 }
 }
// 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.

Answer 1

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

• Sleeper Smith: Let me read through about hoisting first. Much thanks for the input. It's interesting because that's the exact reason I put the query.exec at the end. If it's a standard practice doing then there's no harm putting it at top.
• Sleeper Smith: You know what. I'm marking this an answer. From your link "The reason scoping is so confusing in JavaScript is because it looks like a C-family language." <- exactly what I was looking for. I am a C# developer, and I find JS scope confusing to say the least.
• chovy: You could get rid of the else by doing if ( user ) return found(user); But typically your errors are handled first, so I would do if (!user) return saveNewUser(user); then just drop into the found(user) below that.