CODEWITHSUNDEEP
javascriptnode.jspromisedeferred
Andrew
Andrew

Reputation: 2114

Promise from an array of promises in NodeJS Deferred?

I want to make a module that outputs a set of metrics about the health of my application, such as background queue lengths, response time to service dependencies etc. This is Node JS using Deferred:

var metrics = {
    queueLength: function(def) {
        // .. Do some stuff to resolve the queue length ..
        def.resolve(45); // Example
    }
    // ... more metrics
}
for (i in metrics) {
    def = deferred();
    metrics[i](def);
    promiselist.push(def.promise);
    def.promise(function(result) {
        metrics[i] = result;
    }
}
return deferred(promiselist)(function(result) {
    console.log('All metrics loaded', result, metrics);
});

This produces the output

Metrics loaded [ [Function] ]  { queueLength: [Function] }

When I would have expected:

Metrics loaded [ 45 ]  { queueLength: 45 }

I think I'm doing two things wrong but don't know how to correct them 'properly':

Upvotes: 0

Views: 3378

Answers (2)

Mariusz Nowak
Mariusz Nowak

Reputation: 32840

I agree with most of things Bergi pointed. You shouldn't destroy metrics methods and you should create and return promises internally within them.

There's some other improvements you can do:

There's deferred.map (named as counterpart to [].map) which is dedicated for lists and arrays, you can use it directly to resolve array of promises:

deferred.map(promiselist).then(/* ... */)

Additionally you can improve composition if you use deferred.map in its full and also replace for..in loop:

var result = {}; 
deferred.map(Object.keys(metrics), function (name) {
  return metrics[name]().aside(function (value) { result[name] = value; }); 
}).done(function (resultArr) {
  console.log('All metrics loaded', resultArr, result);
});

Upvotes: 1

Bergi
Bergi

Reputation: 664185

metrics[i] = result;

That's a bad idea. You shouldn't destroy the metrics object, it's properties are supposed to be and to stay functions. If you want an object with the results, build a new one.

The return deferred([array of promises])(group promise) idea doesn't seem to work

From the docs and the code you can see that the library does not support arrays as arguments. You will need to use apply:

deferred.apply(null, promiselist)

I've just realised def is getting reused on each iteration so if I had multiple metrics it would probably only track the last one.

No, it's not getting reused, you're creating a new deferred object each loop turn. You did however forget the var keyword to make the variable local.

There also is a problem with the i variable - it is not in the closure scope of each promise callback, which means that when the callbacks are resolved then the variable will already have the value from the last loop turn. Check JavaScript closure inside loops – simple practical example.

A small design flaw is that the metrics methods do take a deferred as an argument which they will resolve. It would be better if they did take no argument, and return a promise for their result which they create (and manage) on their own.

var metrics = {
    queueLength: function() {
        var def = deferred();
            // .. Do some stuff to resolve the queue length ..
            def.resolve(45); // Example
        return def.promise;
    }
    // ... more metrics
};

var results = {},
    promiselist = [];
for (var p in metrics) {
    promiselist.push( metrics[p]().aside( function(_p) {
        return function(result) {
            results[_p] = result;
        };
    }(p) ) );
}
return deferred.apply(null, promiselist).then(function(resultArr) {
    console.log('All metrics loaded', resultArr, results);
    return results;
});

Upvotes: 0

Related Questions