Understanding promises in node.js for recursive function

Question

I'm trying to use recursive calls to get data out of redis, stopping and returning when the members return null.

So my data is added like this:

SADD parents.<name> <parent1> <parent2>
SADD parents.<parent1> <grandparent1> <grandparent2>
...

And the final data should look like:

[
 {
     label: <name>,
     parents: [
         { label: <parent1>,
           parents: [ {label: <grandparent1>}, {label: <grandparent2> }] },
         { label: <parent2> }
     ]
 }
]

Here's the code I'm messing with (sort of cobbled together from different sources), but I have no idea what I'm doing. Not sure if this code is even useful, I could be way off track.

var redis = require('node-redis');
var r_client = redis.createClient();
var Q = require('q');


function getFromRedis(nodeName){
        var ret = Q.defer();
        r_client.smembers('parents.' + nodeName,function(err,val){
                if (err) ret.reject(err);
                else {
                        var constructedObject={};  //this is our returned object
                        var dependents=[];
                        if (val)
                        {
                                for (var k in val){  //iterate the keys in val
                                        constructedObject.name = val[k];

                                        dependents.push(getFromRedis(val[k])
                                        .then(function(subVal){
                                                constructedObject[k]=subVal;
                                                return ret.promise;
                                        })
                                        );
                                }
                        }
                        else { return [] }

                }
                Q.all(dependents)
                .then(function(){ret.resolve(constructedObject);},ret.reject.bind(ret));

        });
                return ret;
}

getFromRedis( 'greg', function(out) {console.log('Final output: ' + JSON.stringify( out ))} );

I can look at the examples and see theoretically how it's supposed to work, but I can't get my mind around how it should work with the q implementation. Any help would be greatly appreciated.

Did you previously had a callback-based solution? – Bergi Sep 19 '13 at 21:38 — Bergi, Sep 19 '13 at 21:38
No. I tried it, but it was way more complicated. – coding_hero Sep 19 '13 at 21:41 — coding_hero, Sep 19 '13 at 21:41

Bergi · Accepted Answer · 2013-09-20T04:12:57.163

3

Try to be as pure as you can when working with promises. Avoid functions that have side effects, i.e. do manipulate any variables outside of their own scope.
Avoid passing callbacks to functions. Do only pass them to promise methods. You are doing this both with r_client.smembers() and when invoking your getFromRedis method

I can see only one specific mistake that would keep your script from working:

return [];

does not have any effect from the callback. So, ret is never going to be resolved in this case. You would do ret.resolve([]); return; if at all. However, there are better solutions that let you use return again.

To restructure your script, there are two points:

Use the Q.nfcall helper function (and the like) to avoid dealing with callback-style APIs directly. Use then to transform its result then - synchronously returning the tree leaves or a promise for the descendant-getting computations.
Use Q.all first, and then transform its result. Don't add a handler to each dependent, but get the whole result and build the construct in one single step.

function getFromRedis(nodeName){
    return Q.ninvoke(r_client, "smembers", 'parents.' + nodeName).then(function(val) {
        // this is our returned object
        var constructedObject = {label: nodeName};
        if (val) {
            var dependents = val.map(function(par) {
                // get a promise for the next level
                return getFromRedis(nodeName+"."+par.toString());
            });
            return Q.all(dependents).then(function(dependentResults) {
                 constructedObject.parents = dependentResults;
                 return constructedObject;
            });
        } else { 
            return constructedObject; // without parents
        }
    });
}

getFromRedis( 'greg' ).done(function(out) {
    console.log('Final output: ' + JSON.stringify( out ));
});

edited Sep 20 '13 at 04:12

answered Sep 19 '13 at 21:54

Bergi

630,263
148
957
1,375

So much more concise. It all makes sense when I see it working like this. For some reason, when I run it from the command line, though, it never returns any output? It's like it's stuck in a loop. – coding_hero Sep 19 '13 at 22:14
Add some logging to see what `val`s you are getting in the while, maybe there really is. I don't know redis well, it's possible that I didn't construct the queries correctly from the values - see the part of the code [which I just edited](http://stackoverflow.com/posts/18905566/revisions), you might need to adapt it. – Bergi Sep 19 '13 at 22:29
Hmm. Seems like it should work, but I'm getting an error in q.js: `TypeError: Object # – coding_hero Sep 20 '13 at 01:43
The one thing that I did change (I get the error either way): smembers gets buffers back from Redis instead of strings. So I called toString() on 'par'. – coding_hero Sep 20 '13 at 01:57
2

Oh, found [this](https://github.com/kriskowal/q#adapting-node). I changed the Q.nfcall() to this: `return Q.ninvoke(r_client, "smembers", 'parents.' + nodeName).then(function(val) {` I've been banging my head against the wall for the better part of three days trying to figure this out. Thanks so much for all your help. – coding_hero Sep 20 '13 at 03:53
Thank you, that makes only sense. Reflected it in my answer :-) – Bergi Sep 20 '13 at 04:13
If you wouldn't mind one more question -- if I wanted to do another redis call (just a 'get' this time), and stick that in the object also, would it be another `Q.ninvoke(r_client, 'get', 'foo.'+nodeName).done()` call? – coding_hero Sep 20 '13 at 15:59
Yes. And to merge it with the other result, use `Q.all` again. – Bergi Sep 21 '13 at 16:58

Understanding promises in node.js for recursive function

1 Answers1

Linked