I have a function running in a node app that I can't get to work due to my lack of understanding on how to write asynchronous code properly. Below is a function that takes in a profile with emails. I would like to loop through each email and check to see whether that user exists in my database. If they do, I would like to return the callback given and completely exist the function without doing anything else. If the user is not found, I would then like to create a new user based on the information given in the profile, and then return the same callback with the newly created user. As of now, the function works as intended, except that it creates a new user even when a user is already found in my database. (the 'User' variable is defined above and has the 'create' function. Also, I would like to avoid using the 'async' node module if at all possible)
function processProfile(profile, callback) {
var existingUser;
if (profile.emails) {
profile.emails.forEach(function(email) {
console.log("Searching for user with this email:" + email.value);
existingUser = findUserByEmail(email.value);
if (existingUser) {
console.log("Found the existing user");
return callback(null, existingUser);
}
});
if(!existingUser){
console.log("Creating new user");
var newUser = {
id: profile.id,
firstName: profile.name.givenName,
lastName: profile.name.familyName,
email: profile.emails[0].value
};
user.create(newUser, profile.provider, function(err, user) {
if (err) throw err;
return callback(null, user);
});
}
}
}
Is there something wrong with this?
function processProfile(profile, callback) {
var existingUser;
var index = 0;
function processNextEmail() {
if(index >= profile.emails.size()) return; //When we've popped nothing exit
var email = profile.emails[index++];
console.log("Searching for user with this email:" + email.value);
existingUser = findUserByEmail(email.value);
if (existingUser) {
console.log("Found the existing user");
callback(null, existingUser);
processEmail();//recursive call to prcess the next email
} else {
console.log("Creating new user");
var newUser = {
id: profile.id,
firstName: profile.name.givenName,
lastName: profile.name.familyName,
email: profile.emails[0].value
};
user.create(newUser, provider, function(err, user) {
if (err) throw err;
callback(null, user);
processNextEmail();//recursive call to process the next email after creating the user and adding it to the database.
});
}
}
processNextEmail();
}
If you need the recursive logic to not remove the emails, you can do a simple modification involving an indice within the scope of the processProfile() closure.
Also note, the return callback() lines, don't really do anything. Returning from functions that are happening asynchronously is a waste of time. Just call the callback, and then you can call an empty return to skip the rest of the function if you wish, but it is unecessary, unless the return effects the flow of logic.
EDIT: It turns out this is example is too simple to be much more interesting. The code below I used as an example for some people at work who were having trouble grasping async code. The one time I think that it is okay to use sync code in node is for gathering configuration data. Let's pretend that we have configuration stored in a file, that in turn gets the filename from that file, and gathers another layer of configuration data from another file. We can do this two ways, using readFileSyn, or using readFile. The asynchronous version is tricky, because we need to wait for the first step to complete, because we have to grab the filename from the first file, in order to know where the second file is stored. Below is the code for both the sync solution and async solution.
//The synchronous way
function useConfigurationData(configData) {
dosomethinginterestingwith(configData);
}
function getConfigurationData(fileName) {
var fileName2 = fs.readFileSync(fileName);
var configurationData = fs.readFileSync(fileName2);
return configurationData;
}
var fileData = getConfigurationData('someFile');
useConfigurationData(fileData);
//The equivalent async way
function getConfigurationData(fileName, useConfigDataCallBack) {
fs.readFile(fileName, getConfigDataStepTwo);
function getConfigDataStepTwo(err, fileName2) {
fs.readFile(fileName2, getConfigDataStepThree);
}
function getConfigDataStepThree(err, fileData) {
useConfigDataCallBack(fileData);
}
}
getConfigurationData('someFile', useConfigurationData);
Notice that the callback we supply to getConfigurationData is the last step. We could also just rely on the globally defined getConfigurationData function, but passing it in as a callback is better style.
What I like about this syntax, is the code withing the second getConfigurationData function reads in order, very synchronously. But if you follow the flow of logic, it is all being run async. It's easy to read and follows nodes async I/O model. IN the case of configuration data, I think the synchronous option is acceptable, but this is still a good demo of how to get synchronous behavior and syntax(ish) from asynchronous callbacks.