Proper way to use Q promises and recursion

[// ravi]

Hello all,

I have a function that recurses through an input array performing an (async) operation to a DB for each element. I could wire up something ugly like the below, but surely there is a better way? Any advise appreciated.

——————————————————————

input_data = get_input_data();
db_connect()
.then( function(db) { add_entries(db, input_data); } )
.fail( function(e) { console.error(“Bad stuff happened.”); console.error(e); } )
.done();

——————————————————————

And add_entries could be defined something along the lines of (this is not running code, and I am not even sure this will work especially with the way Q promises are being used):

——————————————————————

function add_entries(db, data, promise)
{
if( typeof(promise) === ‘undefined’ )
promise = Q.defer();

if( data.length === 0 )
{
promise.resolve();
return(promise.promise);
}

db.add
(
data.shift(),
function(err)
{
if( err )
promise.reject(err);
else
setTimeout(add_entries.bind(null, db, data, promise), 0);
}
);

return(promise.promise); // meaningful only for first invocation, of course
}

——————————————————————

—ravi

[Domenic Denicola]

I think this is a nice solution to your problem:

function addAllEntries(db, entries) {
if (entries.length === 0) {
return Q();
}

return Q.ninvoke(db, "add", entries.shift()).then(function () {
return addAllEntries(db, entries);
});
}

var inputData = getInputData();

dbConnect()
.then(function (db) {
return addAllEntries(db, inputData);
})
.catch(function (e) {
console.error("Bad stuff happened.", e);
})
.done();

The trick is to do as much in promise space as possible; in particular; writing manual adapters for Node code is no fun, so we use Q.ninvoke to get into promise-land ASAP, and we thus never have to use deferreds, which are usually a sign something weird is going on.

Also note that there's no need for the setTimeout(, 0), since Q guarantees all fulfillment handlers will be run in a clean stack.

Also, by returning the promise that addAllEntries gives, the original caller is notified of any rejections that occur, even after a few recursive calls have happened.

As always, it's helpful to compare to the synchronous code, to see how promises try to maintain that parallel:

function addAllEntries(syncDb, entries) {
if (entries.length === 0) {
return;
}

syncDb.add(entries.shift());
addAllEntries(syncDb, entries);
}

var inputData = getInputData();

var syncDb = dbConnect();
try {
addAllEntries(syncDb, inputData);
} catch (e) {
console.error("Bad stuff happened.", e);
}

[// ravi]

On Nov 16, 2013, at 5:38 PM, Domenic Denicola <dom...@domenicdenicola.com> wrote:
> I think this is a nice solution to your problem:
>
> function addAllEntries(db, entries) {
> if (entries.length === 0) {
> return Q();
> }
>
> return Q.ninvoke(db, "add", entries.shift()).then(function () {
> return addAllEntries(db, entries);
> });
> }
>
> var inputData = getInputData();
>
> dbConnect()
> .then(function (db) {
> return addAllEntries(db, inputData);
> })
> .catch(function (e) {
> console.error("Bad stuff happened.", e);
> })
> .done();
>
> The trick is to do as much in promise space as possible; in particular; writing manual adapters for Node code is no fun, so we use Q.ninvoke to get into promise-land ASAP, and we thus never have to use deferreds, which are usually a sign something weird is going on.
>

Hello Domenic,

that’s a nice solution, thank you. I confess I was being a little lazy, in not using Q.ninvoke() for converting the DB calls to promises. Though I like your approach a lot better than mine, I have one question: my pseudocode uses a single promise rather than returning a chain of promises for each round of recursion (IIUC). The latter is more elegant, for sure, but is there any saving at all in the single promise implementation (assuming it will do what I think it will), that would favour it?

> Also note that there's no need for the setTimeout(, 0), since Q guarantees all fulfillment handlers will be run in a clean stack.

True. I was doing that (setTimeout) since I was not taking advantage of Q.ninvoke.

> Also, by returning the promise that addAllEntries gives, the original caller is notified of any rejections that occur, even after a few recursive calls have happened.

Yes, I missed that in my code.

Thank you for the detailed response. It was both informative and helpful.

—ravi

>
> As always, it's helpful to compare to the synchronous code, to see how promises try to maintain that parallel:
>
> function addAllEntries(syncDb, entries) {
> if (entries.length === 0) {
> return;
> }
>
> syncDb.add(entries.shift());
> addAllEntries(syncDb, entries);
> }
>
> var inputData = getInputData();
>
> var syncDb = dbConnect();
> try {
> addAllEntries(syncDb, inputData);
> } catch (e) {
> console.error("Bad stuff happened.", e);
> }
>

> --
> You received this message because you are subscribed to the Google Groups "Q Continuum (JavaScript)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to q-continuum...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

[Domenic Denicola]

> my pseudocode uses a single promise rather than returning a chain of promises for each round of recursion (IIUC). The latter is more elegant, for sure, but is there any saving at all in the single promise implementation (assuming it will do what I think it will), that would favour it?

That's a good point. Your code is indeed more memory efficient, and since Q is not the fastest promise implementation out there (sadly), it would be more time-efficient as well. I guess it's a tradeoff... if this were something that needed major optimization, indeed your approach would be better.

[// ravi]

On Nov 16, 2013, at 6:12 PM, Domenic Denicola <dom...@domenicdenicola.com> wrote:
>> my pseudocode uses a single promise rather than returning a chain of promises for each round of recursion (IIUC). The latter is more elegant, for sure, but is there any saving at all in the single promise implementation (assuming it will do what I think it will), that would favour it?
>
> That's a good point. Your code is indeed more memory efficient, and since Q is not the fastest promise implementation out there (sadly), it would be more time-efficient as well. I guess it's a tradeoff... if this were something that needed major optimization, indeed your approach would be better.
>

In my case, my code is not long-running and very memory/speed sensitive, so I am going to use your version. Thank you again for your thoughts,

—ravi