jon skulski
jon skulski

Reputation: 2305

Redis, node.js and Javascript Callback Scoping / Flow

I've been exploring javascript more deeply lately, playing around with a node,redis,socket.io,express simple webapp.

I think I've come to a problem that is an example of the fundamental difference between JS and PHP, which is what I've been using the last few years.

I've written this callback and need to return a stack of data from redis

app.get('/deck', function(request, response) { 
  dealer.sort('cards', 'BY', 'deal:*->timestamp', 'GET', 'card:*->card_key',
    function(err, card_keys) {
      var deck = []; 
      for (k in card_keys) { 
        dealer.hget(card_keys[k], 'data', function(err, card_data) { 
          var deal = eval('(' + card_data +')');
          deals.push(cards);
        }); 
      }   
      response.json(deals);
    }   
  );  
});

Now first I thought it was a variable scoping problem so I rewrote with a closure (did I use this term correctly?) which didn't work because I am writing this in synchronous mind set. I realize that this is essentially synchronous and sends the data out before it is collected. The design is wrong.

I can rewrite the function to use socket.io's emit function to send data as it is collected.

But are there design patterns for handling synchronous data? What if I want to show the last 10 cards in order? Do I send the data over anyway and have client side code queue up 10, and trigger an event that sorts and then control display?

Edit: I found this question which essentially mine, except that I would like to figure out how to design this without relying an synchronous library.

Upvotes: 2

Views: 1756

Answers (3)

alessioalex
alessioalex

Reputation: 63653

You aren't doing this properly since the server could respond before the db queries have returned their results.

You should instead use control flow libraries like Step or Async. Learn more about them on dailyjs.

I personally use Step, so here's show I would do it:

app.get('/deck', function(request, response) { 
  dealer.sort('cards', 'BY', 'deal:*->timestamp', 'GET', 'card:*->card_key',
    function(err, card_keys) {
      var deals = [], iterations = Object.keys(card_keys).length;
      // iterations contains the length of 'card_keys'
      Step(
        function queries() {
          var deck = [];
          if (!iterations) { this(null); return; } // if no cards
          for (k in card_keys) {
            dealer.hget(card_keys[k], 'data', function(err, card_data) {
              var deal;
              if (err) {
                throw new Error('Database error');
              } else {
                // we decrease the number of iterations until it gets to 0
                // and then we trigger the callback, since every query has
                // returned its result
                iterations--;
                deal = eval('(' + card_data +')');
                deals.push(cards);
                if (!iterations) { this(deals); } // callback function called
              }
            });
          }
        },
        function render(deals) {
          response.json(deals);
        }
      );
    }
  );
});

Upvotes: 1

malletjo
malletjo

Reputation: 1786

One way to achieve this without using another library is using the "recursive pattern".

You can't do some asynchronious call in a for loop.

card_keys need to be an array.

Some useful link about how to write for loop :

  1. http://tech.richardrodger.com/2011/04/21/node-js-%E2%80%93-how-to-write-a-for-loop-with-callbacks/
  2. http://metaduck.com/post/2675027550/asynchronous-iteration-patterns-in-node-js

EDIT: Like Raynos said, you should use reference counting instead.

I suggest you use a lib and do something like this with async:

...
var deals = []
async.forEach(card_keys, function(err, results) {
    if (err) {
        callback(err);
    } else {
        deals.push(results); // For each card
    }
}, function(err) {
    callback(deals); // When it's completed
});
...

This is how async is doing the forEach

async.forEach = function (arr, iterator, callback) {
    if (!arr.length) {
        return callback();
    }
    var completed = 0;
    _forEach(arr, function (x) {
        iterator(x, function (err) {
            if (err) {
                callback(err);
                callback = function () {};
            }
            else {
                completed += 1;
                if (completed === arr.length) {
                    callback();
                }
            }
        });
    });
};

Edit: Which one is faster : reference counting or recursive pattern

I did 100 basic query :

Resurcive pattern : 7161,7528, 7226 MS

Reference counting: 7515, 7256, 7364 MS

Recursive pattern code :

var req = "select id from membre";

var time = new Date().getTime();

var test = function(value,callback) {
   if(value < 100) {
      client.query(req, function(err, results) {
          test(++value,callback);
      });
   } else {
       callback();
   }
}

test(0, function() {
   var time2 = new Date().getTime();
   console.log(time2-time);
   client.end();
});

Reference counting code :

var req = "select id from membre";

var time = new Date().getTime();

var test = function(callback) {

   var c = 100;
   function next() {
      if(--c === 0) callback();
   }

  for(var i =0; i < c; i++) {
       client.query(req, function(err) {
          next();
       });
   }
};


test(function() {
   var time2 = new Date().getTime();
    console.log(t2-t);
   client.end();
});

Why it's almost the same time ? Since it's mysql we don't gain that much from the "async" from node-js in this situation. See comments.

Upvotes: 2

Raynos
Raynos

Reputation: 169373

function(err, card_keys) {
    var deck = [];
    for (k in card_keys) {
        // whole bunch of async actions
        dealer.hget(card_keys[k], 'data', function(err, card_data) {
            var deal = eval('(' + card_data + ')');
            deals.push(cards);
        });
    }
    // sync action
    response.json(deals);
}

You already know response.json fires before the hget are done.

The answer is reference counting

function(err, card_keys) {
    var deck = [];
    var count = Object.keys(card_keys).length;

    function next() {
        if (--count === 0) {
            response.json(deals.sort(sortThem)); 
        }  
    }

    for (k in card_keys) {
        dealer.hget(card_keys[k], 'data', function(err, card_data) {
            var deal = JSON.parse(card_data);
            deals.push(cards);
            next();
        });
    }
}

You have two other minor problems

  • using eval instead of JSON.parse
  • your deals array is unordered because hget is asynchronous, so you need a sorting function.

Upvotes: 4

Related Questions