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])}));
})
-
1\$\begingroup\$ Welcome to Code Review! Nice job on your first question. \$\endgroup\$Phrancis– Phrancis2016年03月05日 21:17:48 +00:00Commented Mar 5, 2016 at 21:17
1 Answer 1
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.
-
\$\begingroup\$ Thanks for all the feedback, it's really useful! I'll use it all. I just do not understand this part:
{...a, ...p[i]}
, somehow does not seem symmetric? The...
are not real right? \$\endgroup\$PascalVKooten– PascalVKooten2016年03月06日 09:31:49 +00:00Commented Mar 6, 2016 at 9:31 -
\$\begingroup\$ To add: everything worked when just substituting the
let merged
with the "underscore way", but of course I'm still interested in getting it to work like this. \$\endgroup\$PascalVKooten– PascalVKooten2016年03月06日 09:55:36 +00:00Commented Mar 6, 2016 at 9:55 -
\$\begingroup\$ @Dualinity It's actually real. I used to use
...
to mean "everything else", but now it's actually part of JS just recently. \$\endgroup\$Joseph– Joseph2016年03月06日 15:40:40 +00:00Commented Mar 6, 2016 at 15:40
Explore related questions
See similar questions with these tags.