Problem
Steps:
- Get
AMOUNT
from a call to API endpoint/a
, e.g.AMOUNT == 5
(in the code it is set to 2) - Use this AMOUNT to make AMOUNT calls to endpoint
/b
receiving JSON- Result might be:
[{a: 1, b: 2}, {...}, {...}, {...}, {...}]
- Result might be:
- Use this AMOUNT to make AMOUNT calls to endpoint
/c
receiving JSON- Result might be:
[{c: 3, d: 4}, {...}, {...}, {...}, {...}]
- Result might be:
- Merge such that there is a resulting array of JSON objects
- Result might be:
[{a:1, b:2, c: 3, d: 4}, {...}, {...}, {...}, {...}]
- Result might be:
So pretty much it looks like this:
a
/
b c
/
result
I’m really trying to understand how to write nice NodeJS code (coming from Python), so I’m specifically looking for the most elegant way to do this. I hope people can give advice on my current code below, as well as perhaps just suggesting different though clearly better methods. E.g. doing everything in async but still merging into the same result.
I’m also interested in which of the 3 styles of options (1/2/3) is best for writing NodeJS apps.
Promise = require("bluebird");
u = require("underscore")
function get_phase_count() {
return new Promise(function(resolve, reject) {
resolve(2); // AMOUNT == 2
});
}
function get_phase_info(amount) {
console.log(amount) // this would be used but now it is mocked
return new Promise(function(resolve, reject) {
// note that I got this AMOUNT calls solved to deliver this
resolve([ { a: 1, b: 2 }, { a: 1, b: 2 } ]);
});
}
function get_phase_votes(amount) {
console.log(amount) // this would be used but now it is mocked
return new Promise(function(resolve, reject) {
// note that I got this AMOUNT calls solved to deliver this
resolve([ { c: 3, d: 4 }, { c: 3, d: 4 } ]);
});
}
function join_phase_promises(p1, p2) {
return Promise.all([p1, p2]);
};
Using those functions I wrote 3 style options:
// Option 1
get_phase_count().then(function(amount) {
join_phase_promises(get_phase_info(amount), get_phase_votes(amount)).then(function(args) {
console.log(u.map(u.zip(args[0], args[1]), function(x) {return u.extend(x[0], x[1])}));
})
});
// Option 2
get_phase_count()
.then(function(amount) {
return join_phase_promises(get_phase_info(amount), get_phase_votes(amount));
})
.then(function(args) {
console.log(u.map(u.zip(args[0], args[1]), function(x) {return u.extend(x[0], x[1])}));
})
// Option 3
get_phase_count().then(function(amount) {
return join_phase_promises(get_phase_info(amount), get_phase_votes(amount));
}).then(function(args) {
console.log(u.map(u.zip(args[0], args[1]), function(x) {return u.extend(x[0], x[1])}));
})
Solution
Option 1 defeats the whole purpose of Promises. You are nesting the callbacks. Option 2 and 3 are not different, just how they’re formatted. That’s up to personal preferences.
Now going with the fact that get_phase_count
, get_phase_info
and get_phase_votes
are wrapper functions to API calls, let’s not touch those. That leaves us with join_phase_promises
. join_phase_promises
is unnecessary as it’s just a wrapper for Promise.all
which you can simply use directly.
Now over to code styles, JS follows camelCase convention. This makes your function names, which are underscore-cased, look like outcasts. Suggesting you use camelCase instead.
If you happen to use Node.js 4+ (current version is 5.7), the native Promise
is already available. No need to use Bluebird for that. You can also start using arrow functions to make your callbacks a bit more compact. Lastly, JS has a bunch of array operations and recently included destructuring. There’s little need to use underscore.
If I were to write your function, it would go like
getPhaseCount().then(amount => Promise.all([
getPhaseInfo(amount),
getPhaseVotes(amount)
])).then(args => {
let merged = args.reduce((p,c) => c.map((a, i) => ({...a, ...p[i]}) ), []);
console.log(merged);
});
The above logic took me a while to figure out using only native operations, but what it does is to allow us to see the previous and current values using reduce
. This way, on each item, map essentially creates a new array containing a merged version of the previous and current.