Function isn't passing correct return value from a simple function in nodejs app

Question

I have the following code and am attempting to make a Turntable bot using node.js. this piece of code says when a user types "q+" we have to make sure its not already on the queue, that its not already DJing, and if it meets those 2 requirements, add them to the queue. Otherwise if it doesnt meet one of those first 2 criteria, tell the user and do not touch the queue.

My problem is the "isCurrentDJ(userId)". When I pass a userId through that function, the function gives me the correct answer. However the function ALWAYS passes back "false" even when the answer is "true" and the console.log() function within the isCurrentDJ(userId) function proves so.

I am not the most js-savvy person, so I think this may be a variable scope issue. But I am really not sure and have been struggling with it for hours! Any help would be greatly appreciated. Thanks!

// When someone speaks, listen to see if it is one of the q commands
bot.on('speak', function (data) {
var name = data.name;
var text = data.text;
var userId = data.userid;

// q+ :: Add to Queue
if (text.match(/^q\+$/)) {

  //Index is the position in the queue that this person's name is found.
  //If its not found, -1 is returned.
  var index = queue.indexOf(name);

  //Function to determine if the user is currently a DJ
    function isCurrentDJ(user_id, callback){
      bot.roomInfo(false, function (data) {
        var djList = data.room.metadata.djs;
        for (i = 0; i < djList.length; i++){
          if (djList[i] == user_id){
            console.log('recognized as a DJ'); //Consistently printed!
            callback(true);
          }
        }
        callback(false);
      });
    }


  isCurrentDJ(userId, function(isDJ) {
    //If the user is already in the queue
    if(index > -1){
      //Tell them they are already in there
      bot.speak('You are already on the list');
    } else if(isDJ){
      //Otherwise if they are already a DJ tell them that
      bot.speak('You are already a DJ, '+name);
    }else{
      //Otherise if they are not in the queue add user to end of queue
      queue.push(name);
      //Tell them about it and the updated q
      bot.speak(name+' has been added to queue.');
    }
  });

}

Laurent Perrin · Accepted Answer · 2012-08-06 01:13:35Z

1

Your problem is that bot.roomInfo is an asynchronous function.

When you call it, it immediately returns and currDJ is still false. A little while later, the callback (function(data) {...) is called. Most of node.js's API are async so that your code never blocks.

Here's how you should rewrite your code:

// When someone speaks, listen to see if it is one of the q commands
bot.on('speak', function (data) {
   var name = data.name;
   var text = data.text;
   var userId = data.userid;

   // q+ :: Add to Queue
  if (text.match(/^q\+$/)) {

  //Index is the position in the queue that this person's name is found.
  //If its not found, -1 is returned.
  var index = queue.indexOf(name);

  //Function to determine if the user is currently a DJ
    function testCurrentDJ(user_id, cb){

      bot.roomInfo(false, function (data) {
        var djList = data.room.metadata.djs;
        for (i = 0; i < djList.length; i++){
          if (djList[i] == user_id){
            console.log('recognized as a DJ'); //Consistently printed!
            return cb(true);
          }
        }

        cb(false);
      });
    }

  //If the user is already in the queue
  if(index > -1){
    //Tell them they are already in there
    bot.speak('You are already on the list');
    return;
  }

  testCurrentDJ(userId, function(isDJ) {
      //Otherwise if they are already a DJ tell them that
      if(isDJ) {
        bot.speak('You are already a DJ, '+name);
      } else {
       //Otherise if they are not in the queue add user to end of queue
       queue.push(name);
       //Tell them about it and the updated q
       bot.speak(name+' has been added to queue. Is Current DJ? '+isDJ);
  }

    })
}

I've just updated your code to show you the basic idea. In node.js's API, the first argument of callbacks is usually an error object that is null if everything went fine.

edited Aug 6, 2012 at 1:13

answered Aug 6, 2012 at 0:20

Laurent Perrin

14.9k5 gold badges53 silver badges49 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

ARH3 Over a year ago

Thanks Laurent. I think I understand what is going on here... but it looks like if a user is already in the queue array they will see the bot say "You are already on the list" and then be added to the list anyway. Is there a way to avoid that in the code too? Since 'index' is also a global variable I am not sure how nicely it would play within the isCurrentDJ function.

Laurent Perrin Over a year ago

I've updated the code to immediately return without calling testCurrentDJ if user is already in queue. Another thing is that things always happen sequentially: a callback will never be triggered before you return.

ARH3 Over a year ago

Hi I just updated my original code to a mix of what you gave me and what I started with. As I summarized in the changes note, it now correctly identifies when I am DJing, but for some reason does a second turn through the function and adds the user to the queue the second time around. Any suggestions here? While it appears your way works, for my own sanity I would prefer using 3 if/else statements instead of an if/return combo followed by 2 more if/else statements later.

ARH3 Over a year ago

Actually what I just said is slightly false. On the first turn it adds me to the queue, and then on the second turn it says I am already a DJ. So what I think is happening is the function finishes thinking I am not a DJ, but then once the async function finishes it realizes I am a DJ and so in the end it ends up doing both. How would I limit to just the correct answer?

Laurent Perrin Over a year ago

OK, there was just one last stupid mistake. In the callback of bot.roomInfo, we loop over room.metadata.djs, and call true if we find a match. Problem is, once the iteration is over, we call false no matter what. The fix is to change to "return callback(true);".

|

Halcyon · Accepted Answer · 2012-08-06 00:10:55Z

0

Is bot.roomInfo perhaps an asynchronous function? If so the value of currDJ will be set to true, but too late because you've already returned it. You can no operate on the value of currDJ untill that callback is called.

How familiar are you with the concept of asynchronous functions?

answered Aug 6, 2012 at 0:10

Halcyon

57.7k10 gold badges93 silver badges131 bronze badges

2 Comments

ARH3 Over a year ago

Not familiar at all to be honest... but I read about that somewhere else and am thinking that that is probably the issue. Would it be a major code adjustment? Perhaps a tutorial would be a good place for me to start.

Halcyon Over a year ago

It's probably too complicated to try and explain here. Look at generic AJAX stuff, you might find some answers there.

Collectives™ on Stack Overflow

Function isn't passing correct return value from a simple function in nodejs app

2 Answers 2

6 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related