How to optimize this block of code since its async
var orderItems = userData.shoppingcart;
var totalPrice = 0;
userData.shoppingcart.forEach(function(itemName, i)
_data.read('menuitems', itemName, function(err, itemData)
if(!err && itemData)
totalPrice += itemData.price;
if(++i == userData.shoppingcart.length)
// Only here when all the itemNames have been read I should continue
);
);
As you can see, the call to _data.read is async, because I am reading from a file.
But I need to wait to have all the files read, so I can have the totalPrice calculated. That's why I place that condition [ ++i == userData.shoppingcart.length ].
I am new to javascript and nodejs in general, never was very good programmar at it, but anyway my point is that I am sure this isn't a good approach, what if both files are readed at same time, and then that condition is never ran or the calculation of totalPrice is badly done?
Can someone give me please some guidance on this?
Thank you in advance!
javascript node.js asynchronous conditional
add a comment |
var orderItems = userData.shoppingcart;
var totalPrice = 0;
userData.shoppingcart.forEach(function(itemName, i)
_data.read('menuitems', itemName, function(err, itemData)
if(!err && itemData)
totalPrice += itemData.price;
if(++i == userData.shoppingcart.length)
// Only here when all the itemNames have been read I should continue
);
);
As you can see, the call to _data.read is async, because I am reading from a file.
But I need to wait to have all the files read, so I can have the totalPrice calculated. That's why I place that condition [ ++i == userData.shoppingcart.length ].
I am new to javascript and nodejs in general, never was very good programmar at it, but anyway my point is that I am sure this isn't a good approach, what if both files are readed at same time, and then that condition is never ran or the calculation of totalPrice is badly done?
Can someone give me please some guidance on this?
Thank you in advance!
javascript node.js asynchronous conditional
1
"what if both files are read at same time" - isn't that exactly what you want?
– Bergi
Nov 11 '18 at 19:23
1
Basically, yes that's the idea - counting how many of the tasks are done, and then continue. There are many pitfalls though, like taking errors into account correctly or the edge case of 0 iterations (like on an empty input), so you better use a some proven existing code instead of reinventing the wheel. Learn about promises andPromise.all
.
– Bergi
Nov 11 '18 at 19:25
Yes they can all read at the same time, but I want the flux to continue ONLY when all have been read, so I have the right and exact amount of totalPrice variable. Yes I have heard about Promise, but don't know how to use it, will do some search on this, thanks @Bergi
– TiagoM
Nov 11 '18 at 19:34
add a comment |
var orderItems = userData.shoppingcart;
var totalPrice = 0;
userData.shoppingcart.forEach(function(itemName, i)
_data.read('menuitems', itemName, function(err, itemData)
if(!err && itemData)
totalPrice += itemData.price;
if(++i == userData.shoppingcart.length)
// Only here when all the itemNames have been read I should continue
);
);
As you can see, the call to _data.read is async, because I am reading from a file.
But I need to wait to have all the files read, so I can have the totalPrice calculated. That's why I place that condition [ ++i == userData.shoppingcart.length ].
I am new to javascript and nodejs in general, never was very good programmar at it, but anyway my point is that I am sure this isn't a good approach, what if both files are readed at same time, and then that condition is never ran or the calculation of totalPrice is badly done?
Can someone give me please some guidance on this?
Thank you in advance!
javascript node.js asynchronous conditional
var orderItems = userData.shoppingcart;
var totalPrice = 0;
userData.shoppingcart.forEach(function(itemName, i)
_data.read('menuitems', itemName, function(err, itemData)
if(!err && itemData)
totalPrice += itemData.price;
if(++i == userData.shoppingcart.length)
// Only here when all the itemNames have been read I should continue
);
);
As you can see, the call to _data.read is async, because I am reading from a file.
But I need to wait to have all the files read, so I can have the totalPrice calculated. That's why I place that condition [ ++i == userData.shoppingcart.length ].
I am new to javascript and nodejs in general, never was very good programmar at it, but anyway my point is that I am sure this isn't a good approach, what if both files are readed at same time, and then that condition is never ran or the calculation of totalPrice is badly done?
Can someone give me please some guidance on this?
Thank you in advance!
javascript node.js asynchronous conditional
javascript node.js asynchronous conditional
asked Nov 11 '18 at 19:08
TiagoM
1,48542560
1,48542560
1
"what if both files are read at same time" - isn't that exactly what you want?
– Bergi
Nov 11 '18 at 19:23
1
Basically, yes that's the idea - counting how many of the tasks are done, and then continue. There are many pitfalls though, like taking errors into account correctly or the edge case of 0 iterations (like on an empty input), so you better use a some proven existing code instead of reinventing the wheel. Learn about promises andPromise.all
.
– Bergi
Nov 11 '18 at 19:25
Yes they can all read at the same time, but I want the flux to continue ONLY when all have been read, so I have the right and exact amount of totalPrice variable. Yes I have heard about Promise, but don't know how to use it, will do some search on this, thanks @Bergi
– TiagoM
Nov 11 '18 at 19:34
add a comment |
1
"what if both files are read at same time" - isn't that exactly what you want?
– Bergi
Nov 11 '18 at 19:23
1
Basically, yes that's the idea - counting how many of the tasks are done, and then continue. There are many pitfalls though, like taking errors into account correctly or the edge case of 0 iterations (like on an empty input), so you better use a some proven existing code instead of reinventing the wheel. Learn about promises andPromise.all
.
– Bergi
Nov 11 '18 at 19:25
Yes they can all read at the same time, but I want the flux to continue ONLY when all have been read, so I have the right and exact amount of totalPrice variable. Yes I have heard about Promise, but don't know how to use it, will do some search on this, thanks @Bergi
– TiagoM
Nov 11 '18 at 19:34
1
1
"what if both files are read at same time" - isn't that exactly what you want?
– Bergi
Nov 11 '18 at 19:23
"what if both files are read at same time" - isn't that exactly what you want?
– Bergi
Nov 11 '18 at 19:23
1
1
Basically, yes that's the idea - counting how many of the tasks are done, and then continue. There are many pitfalls though, like taking errors into account correctly or the edge case of 0 iterations (like on an empty input), so you better use a some proven existing code instead of reinventing the wheel. Learn about promises and
Promise.all
.– Bergi
Nov 11 '18 at 19:25
Basically, yes that's the idea - counting how many of the tasks are done, and then continue. There are many pitfalls though, like taking errors into account correctly or the edge case of 0 iterations (like on an empty input), so you better use a some proven existing code instead of reinventing the wheel. Learn about promises and
Promise.all
.– Bergi
Nov 11 '18 at 19:25
Yes they can all read at the same time, but I want the flux to continue ONLY when all have been read, so I have the right and exact amount of totalPrice variable. Yes I have heard about Promise, but don't know how to use it, will do some search on this, thanks @Bergi
– TiagoM
Nov 11 '18 at 19:34
Yes they can all read at the same time, but I want the flux to continue ONLY when all have been read, so I have the right and exact amount of totalPrice variable. Yes I have heard about Promise, but don't know how to use it, will do some search on this, thanks @Bergi
– TiagoM
Nov 11 '18 at 19:34
add a comment |
2 Answers
2
active
oldest
votes
Given that you don't specify what context this is in, I'm going to make a few assumptions:
- I assume that
_data.read()
doesn't already support returning a promise. - I assume that this code needs to either call a callback function or return a promise.
My (somewhat naive) approach to this would be to:
- Map the
orderItems
into Promises for each price of that item. - Map the result into a total
- Return that resulting promise OR call the callback.
Here's an annotated example of how you might do it:
// Promise.all takes an array of promises and returns
// a new promise that completes when all the promises in the array are complete.
const promiseOfPrices = Promise.all(
// Here we map all the items in the shopping cart into promises for each of their prices
userData.shoppingcart.map(
// The Promise object takes a single function that it will immediatly call with functions to resolve or
// reject the promise. I suggest reading up on Promises if you're not familiar with them.
itemName => new Promise((resolve, reject) =>
// Here, we have a `reject` and `resolve` function that will each complete the new promise,
// either in success or error respectfully.
// Do the actual read of your file or database or whatever
_data.read('menuitems', itemName, (err, itemData) =>
// If there was an error, reject this promise.
if (err) reject(err);
// Otherwise, we're successful and we resolve with the price of the item
else resolve(itemData.price);
);
)
)
);
// Now, we have a promise (promiseOfPrices) for all the prices of the items in the cart. We use `then` which will
// perform a transform on the result, much like the `map` function on an Array.
const promiseOfTotal = promiseOfPrices.then(
// Here we use the `Array.reduce` function to succinctly sum the values in the array.
arrayOfCartItemPrices => arrayOfCartItemPrices.reduce(
// For each item, reduce calls our function with the current sum and an item in the array. We produce a new
// sum by adding the sum to the item price.
(sum, itemPrice) => sum + itemPrice,
// This is the initial value for sum, 0.
0
)
);
If you can return a promise, and you just want to return the total, then
return promiseOfTotal;
If you have a callback that expects (err, result), then do something like this:
promiseOfTotal.then(
result => callback(null, result),
error => callback(error, null),
)
If you need to do more work on the result, you can do so with another then:
promiseOfTotal.then(
priceSum =>
// Do work here
,
// Optionally handle errors here:
error =>
// Do error handling here.
)
Note that by using promises, arrow functions, and array comprehensions (map
and reduce
) we avoid complex and hard to follow mutation of variables and loops. This is a "functional" style of programming, and while somewhat harder to learn, is generally safer and cleaner than the alternatives. I suggest taking the time to understand how this works, as it'll help you write code that's less likely to have bugs when you're dealing with complex things like asynchronicity.
Finally, I haven't run this code. It may well have a bug or two. Feel free to ask for clarification or if it doesn't work.
Best of luck!
P.S. I didn't go in to using async
/await
because I think it would be less clear than using the Promises directly, and the use of Promise.all
is needed for the parallelism anyways. It's absolutely possible to use them to good effect here, but I'll leave that as an exercise to the OP.
Hi @YonaAppletree thank you so much for your detailed answer, never used promises, it's a good time to learn them! I believe I got the logic and understood in general your code. I just got a question, promiseOfPrices will be an array of all the prices right? In this case, if there was a single reject, I will know because the length of the array 'promiseOfPrices' will be different than the one I expect it to be, is that right? Just to clarify, I am accepting your detailed answer now :)
– TiagoM
Nov 13 '18 at 10:38
@TiagoM Promises are a really great abstraction, and I do suggest reading the docs, since there is more than I can say in one SO post. Answering your question: no.Promise.all
produces a new promise which will resolve when all promises passed to it resolve (success) or reject when any one of them reject (fail). So,promiseOfPrices
will be a Promise that resolves with an array of all prices. It will never resolve with an array of a different length. If any single one fails, thepromiseOfPrices
promise will reject (fail) with that first error.
– Yona Appletree
Nov 13 '18 at 20:39
The google introductory tutorial on Promises seems pretty good: developers.google.com/web/fundamentals/primers/promises
– Yona Appletree
Nov 13 '18 at 20:43
Put another way, if you want to know if anything went wrong, look at the example I gave about doing more work, with two arguments to.then()
or you could use.catch()
to catch errors.
– Yona Appletree
Nov 13 '18 at 20:47
Hello there @YonaAppletree just wanted to pass by and say thank you for the docs and explanation! I got it working finally the way I need, using promises.All and Then clause, also I will read the doc you sent me because it sounds very well documented! Thank you again for all the help and your time! All the best ;)
– TiagoM
Nov 14 '18 at 1:19
add a comment |
This is how you can read the items in sequence using promises (async/await flavour):
var orderItems = userData.shoppingcart;
let totalPrice = 0;
for (let itemName of userData.shoppingcart)
const itemData = await _data.read('menuitems', itemName);
totalPrice += itemData.price;
This example assumes that _data.read
supports async/await
. If it doesn't, however, it can "promisified" using the promisify
function in nodejs' util module
…assuming that_data.read
supports promises.
– Bergi
Nov 11 '18 at 21:55
Yeah, and this runs in sequence, rather than parallel, which isn't likely desirable. One of my biggest issues withasync
/await
is that it seems to encourage less parallel code.
– Yona Appletree
Nov 12 '18 at 19:31
@bergi yes,async/await
is assumed in this case, will add it to the answer.
– Christian
Nov 12 '18 at 19:36
@YonaAppletree I find it more often than not that it's desirable to optimise for readability rather than performance.
– Christian
Nov 12 '18 at 19:39
1
@Christian Of course. But there's performance and there's performance. In this case, you're taking something that might be O(1) and turning it into O(N). That's just not the same as something like the intermediate arrays created when using comprehensions. O(1) vs O(N) is huge, especially if there's network or file reads or whatever. It's at least worth considering carefully.
– Yona Appletree
Nov 13 '18 at 2:06
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "1"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53252200%2fhow-to-optimize-this-block-of-code-since-its-async%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
Given that you don't specify what context this is in, I'm going to make a few assumptions:
- I assume that
_data.read()
doesn't already support returning a promise. - I assume that this code needs to either call a callback function or return a promise.
My (somewhat naive) approach to this would be to:
- Map the
orderItems
into Promises for each price of that item. - Map the result into a total
- Return that resulting promise OR call the callback.
Here's an annotated example of how you might do it:
// Promise.all takes an array of promises and returns
// a new promise that completes when all the promises in the array are complete.
const promiseOfPrices = Promise.all(
// Here we map all the items in the shopping cart into promises for each of their prices
userData.shoppingcart.map(
// The Promise object takes a single function that it will immediatly call with functions to resolve or
// reject the promise. I suggest reading up on Promises if you're not familiar with them.
itemName => new Promise((resolve, reject) =>
// Here, we have a `reject` and `resolve` function that will each complete the new promise,
// either in success or error respectfully.
// Do the actual read of your file or database or whatever
_data.read('menuitems', itemName, (err, itemData) =>
// If there was an error, reject this promise.
if (err) reject(err);
// Otherwise, we're successful and we resolve with the price of the item
else resolve(itemData.price);
);
)
)
);
// Now, we have a promise (promiseOfPrices) for all the prices of the items in the cart. We use `then` which will
// perform a transform on the result, much like the `map` function on an Array.
const promiseOfTotal = promiseOfPrices.then(
// Here we use the `Array.reduce` function to succinctly sum the values in the array.
arrayOfCartItemPrices => arrayOfCartItemPrices.reduce(
// For each item, reduce calls our function with the current sum and an item in the array. We produce a new
// sum by adding the sum to the item price.
(sum, itemPrice) => sum + itemPrice,
// This is the initial value for sum, 0.
0
)
);
If you can return a promise, and you just want to return the total, then
return promiseOfTotal;
If you have a callback that expects (err, result), then do something like this:
promiseOfTotal.then(
result => callback(null, result),
error => callback(error, null),
)
If you need to do more work on the result, you can do so with another then:
promiseOfTotal.then(
priceSum =>
// Do work here
,
// Optionally handle errors here:
error =>
// Do error handling here.
)
Note that by using promises, arrow functions, and array comprehensions (map
and reduce
) we avoid complex and hard to follow mutation of variables and loops. This is a "functional" style of programming, and while somewhat harder to learn, is generally safer and cleaner than the alternatives. I suggest taking the time to understand how this works, as it'll help you write code that's less likely to have bugs when you're dealing with complex things like asynchronicity.
Finally, I haven't run this code. It may well have a bug or two. Feel free to ask for clarification or if it doesn't work.
Best of luck!
P.S. I didn't go in to using async
/await
because I think it would be less clear than using the Promises directly, and the use of Promise.all
is needed for the parallelism anyways. It's absolutely possible to use them to good effect here, but I'll leave that as an exercise to the OP.
Hi @YonaAppletree thank you so much for your detailed answer, never used promises, it's a good time to learn them! I believe I got the logic and understood in general your code. I just got a question, promiseOfPrices will be an array of all the prices right? In this case, if there was a single reject, I will know because the length of the array 'promiseOfPrices' will be different than the one I expect it to be, is that right? Just to clarify, I am accepting your detailed answer now :)
– TiagoM
Nov 13 '18 at 10:38
@TiagoM Promises are a really great abstraction, and I do suggest reading the docs, since there is more than I can say in one SO post. Answering your question: no.Promise.all
produces a new promise which will resolve when all promises passed to it resolve (success) or reject when any one of them reject (fail). So,promiseOfPrices
will be a Promise that resolves with an array of all prices. It will never resolve with an array of a different length. If any single one fails, thepromiseOfPrices
promise will reject (fail) with that first error.
– Yona Appletree
Nov 13 '18 at 20:39
The google introductory tutorial on Promises seems pretty good: developers.google.com/web/fundamentals/primers/promises
– Yona Appletree
Nov 13 '18 at 20:43
Put another way, if you want to know if anything went wrong, look at the example I gave about doing more work, with two arguments to.then()
or you could use.catch()
to catch errors.
– Yona Appletree
Nov 13 '18 at 20:47
Hello there @YonaAppletree just wanted to pass by and say thank you for the docs and explanation! I got it working finally the way I need, using promises.All and Then clause, also I will read the doc you sent me because it sounds very well documented! Thank you again for all the help and your time! All the best ;)
– TiagoM
Nov 14 '18 at 1:19
add a comment |
Given that you don't specify what context this is in, I'm going to make a few assumptions:
- I assume that
_data.read()
doesn't already support returning a promise. - I assume that this code needs to either call a callback function or return a promise.
My (somewhat naive) approach to this would be to:
- Map the
orderItems
into Promises for each price of that item. - Map the result into a total
- Return that resulting promise OR call the callback.
Here's an annotated example of how you might do it:
// Promise.all takes an array of promises and returns
// a new promise that completes when all the promises in the array are complete.
const promiseOfPrices = Promise.all(
// Here we map all the items in the shopping cart into promises for each of their prices
userData.shoppingcart.map(
// The Promise object takes a single function that it will immediatly call with functions to resolve or
// reject the promise. I suggest reading up on Promises if you're not familiar with them.
itemName => new Promise((resolve, reject) =>
// Here, we have a `reject` and `resolve` function that will each complete the new promise,
// either in success or error respectfully.
// Do the actual read of your file or database or whatever
_data.read('menuitems', itemName, (err, itemData) =>
// If there was an error, reject this promise.
if (err) reject(err);
// Otherwise, we're successful and we resolve with the price of the item
else resolve(itemData.price);
);
)
)
);
// Now, we have a promise (promiseOfPrices) for all the prices of the items in the cart. We use `then` which will
// perform a transform on the result, much like the `map` function on an Array.
const promiseOfTotal = promiseOfPrices.then(
// Here we use the `Array.reduce` function to succinctly sum the values in the array.
arrayOfCartItemPrices => arrayOfCartItemPrices.reduce(
// For each item, reduce calls our function with the current sum and an item in the array. We produce a new
// sum by adding the sum to the item price.
(sum, itemPrice) => sum + itemPrice,
// This is the initial value for sum, 0.
0
)
);
If you can return a promise, and you just want to return the total, then
return promiseOfTotal;
If you have a callback that expects (err, result), then do something like this:
promiseOfTotal.then(
result => callback(null, result),
error => callback(error, null),
)
If you need to do more work on the result, you can do so with another then:
promiseOfTotal.then(
priceSum =>
// Do work here
,
// Optionally handle errors here:
error =>
// Do error handling here.
)
Note that by using promises, arrow functions, and array comprehensions (map
and reduce
) we avoid complex and hard to follow mutation of variables and loops. This is a "functional" style of programming, and while somewhat harder to learn, is generally safer and cleaner than the alternatives. I suggest taking the time to understand how this works, as it'll help you write code that's less likely to have bugs when you're dealing with complex things like asynchronicity.
Finally, I haven't run this code. It may well have a bug or two. Feel free to ask for clarification or if it doesn't work.
Best of luck!
P.S. I didn't go in to using async
/await
because I think it would be less clear than using the Promises directly, and the use of Promise.all
is needed for the parallelism anyways. It's absolutely possible to use them to good effect here, but I'll leave that as an exercise to the OP.
Hi @YonaAppletree thank you so much for your detailed answer, never used promises, it's a good time to learn them! I believe I got the logic and understood in general your code. I just got a question, promiseOfPrices will be an array of all the prices right? In this case, if there was a single reject, I will know because the length of the array 'promiseOfPrices' will be different than the one I expect it to be, is that right? Just to clarify, I am accepting your detailed answer now :)
– TiagoM
Nov 13 '18 at 10:38
@TiagoM Promises are a really great abstraction, and I do suggest reading the docs, since there is more than I can say in one SO post. Answering your question: no.Promise.all
produces a new promise which will resolve when all promises passed to it resolve (success) or reject when any one of them reject (fail). So,promiseOfPrices
will be a Promise that resolves with an array of all prices. It will never resolve with an array of a different length. If any single one fails, thepromiseOfPrices
promise will reject (fail) with that first error.
– Yona Appletree
Nov 13 '18 at 20:39
The google introductory tutorial on Promises seems pretty good: developers.google.com/web/fundamentals/primers/promises
– Yona Appletree
Nov 13 '18 at 20:43
Put another way, if you want to know if anything went wrong, look at the example I gave about doing more work, with two arguments to.then()
or you could use.catch()
to catch errors.
– Yona Appletree
Nov 13 '18 at 20:47
Hello there @YonaAppletree just wanted to pass by and say thank you for the docs and explanation! I got it working finally the way I need, using promises.All and Then clause, also I will read the doc you sent me because it sounds very well documented! Thank you again for all the help and your time! All the best ;)
– TiagoM
Nov 14 '18 at 1:19
add a comment |
Given that you don't specify what context this is in, I'm going to make a few assumptions:
- I assume that
_data.read()
doesn't already support returning a promise. - I assume that this code needs to either call a callback function or return a promise.
My (somewhat naive) approach to this would be to:
- Map the
orderItems
into Promises for each price of that item. - Map the result into a total
- Return that resulting promise OR call the callback.
Here's an annotated example of how you might do it:
// Promise.all takes an array of promises and returns
// a new promise that completes when all the promises in the array are complete.
const promiseOfPrices = Promise.all(
// Here we map all the items in the shopping cart into promises for each of their prices
userData.shoppingcart.map(
// The Promise object takes a single function that it will immediatly call with functions to resolve or
// reject the promise. I suggest reading up on Promises if you're not familiar with them.
itemName => new Promise((resolve, reject) =>
// Here, we have a `reject` and `resolve` function that will each complete the new promise,
// either in success or error respectfully.
// Do the actual read of your file or database or whatever
_data.read('menuitems', itemName, (err, itemData) =>
// If there was an error, reject this promise.
if (err) reject(err);
// Otherwise, we're successful and we resolve with the price of the item
else resolve(itemData.price);
);
)
)
);
// Now, we have a promise (promiseOfPrices) for all the prices of the items in the cart. We use `then` which will
// perform a transform on the result, much like the `map` function on an Array.
const promiseOfTotal = promiseOfPrices.then(
// Here we use the `Array.reduce` function to succinctly sum the values in the array.
arrayOfCartItemPrices => arrayOfCartItemPrices.reduce(
// For each item, reduce calls our function with the current sum and an item in the array. We produce a new
// sum by adding the sum to the item price.
(sum, itemPrice) => sum + itemPrice,
// This is the initial value for sum, 0.
0
)
);
If you can return a promise, and you just want to return the total, then
return promiseOfTotal;
If you have a callback that expects (err, result), then do something like this:
promiseOfTotal.then(
result => callback(null, result),
error => callback(error, null),
)
If you need to do more work on the result, you can do so with another then:
promiseOfTotal.then(
priceSum =>
// Do work here
,
// Optionally handle errors here:
error =>
// Do error handling here.
)
Note that by using promises, arrow functions, and array comprehensions (map
and reduce
) we avoid complex and hard to follow mutation of variables and loops. This is a "functional" style of programming, and while somewhat harder to learn, is generally safer and cleaner than the alternatives. I suggest taking the time to understand how this works, as it'll help you write code that's less likely to have bugs when you're dealing with complex things like asynchronicity.
Finally, I haven't run this code. It may well have a bug or two. Feel free to ask for clarification or if it doesn't work.
Best of luck!
P.S. I didn't go in to using async
/await
because I think it would be less clear than using the Promises directly, and the use of Promise.all
is needed for the parallelism anyways. It's absolutely possible to use them to good effect here, but I'll leave that as an exercise to the OP.
Given that you don't specify what context this is in, I'm going to make a few assumptions:
- I assume that
_data.read()
doesn't already support returning a promise. - I assume that this code needs to either call a callback function or return a promise.
My (somewhat naive) approach to this would be to:
- Map the
orderItems
into Promises for each price of that item. - Map the result into a total
- Return that resulting promise OR call the callback.
Here's an annotated example of how you might do it:
// Promise.all takes an array of promises and returns
// a new promise that completes when all the promises in the array are complete.
const promiseOfPrices = Promise.all(
// Here we map all the items in the shopping cart into promises for each of their prices
userData.shoppingcart.map(
// The Promise object takes a single function that it will immediatly call with functions to resolve or
// reject the promise. I suggest reading up on Promises if you're not familiar with them.
itemName => new Promise((resolve, reject) =>
// Here, we have a `reject` and `resolve` function that will each complete the new promise,
// either in success or error respectfully.
// Do the actual read of your file or database or whatever
_data.read('menuitems', itemName, (err, itemData) =>
// If there was an error, reject this promise.
if (err) reject(err);
// Otherwise, we're successful and we resolve with the price of the item
else resolve(itemData.price);
);
)
)
);
// Now, we have a promise (promiseOfPrices) for all the prices of the items in the cart. We use `then` which will
// perform a transform on the result, much like the `map` function on an Array.
const promiseOfTotal = promiseOfPrices.then(
// Here we use the `Array.reduce` function to succinctly sum the values in the array.
arrayOfCartItemPrices => arrayOfCartItemPrices.reduce(
// For each item, reduce calls our function with the current sum and an item in the array. We produce a new
// sum by adding the sum to the item price.
(sum, itemPrice) => sum + itemPrice,
// This is the initial value for sum, 0.
0
)
);
If you can return a promise, and you just want to return the total, then
return promiseOfTotal;
If you have a callback that expects (err, result), then do something like this:
promiseOfTotal.then(
result => callback(null, result),
error => callback(error, null),
)
If you need to do more work on the result, you can do so with another then:
promiseOfTotal.then(
priceSum =>
// Do work here
,
// Optionally handle errors here:
error =>
// Do error handling here.
)
Note that by using promises, arrow functions, and array comprehensions (map
and reduce
) we avoid complex and hard to follow mutation of variables and loops. This is a "functional" style of programming, and while somewhat harder to learn, is generally safer and cleaner than the alternatives. I suggest taking the time to understand how this works, as it'll help you write code that's less likely to have bugs when you're dealing with complex things like asynchronicity.
Finally, I haven't run this code. It may well have a bug or two. Feel free to ask for clarification or if it doesn't work.
Best of luck!
P.S. I didn't go in to using async
/await
because I think it would be less clear than using the Promises directly, and the use of Promise.all
is needed for the parallelism anyways. It's absolutely possible to use them to good effect here, but I'll leave that as an exercise to the OP.
edited Nov 11 '18 at 21:53
answered Nov 11 '18 at 21:48
Yona Appletree
3,50552635
3,50552635
Hi @YonaAppletree thank you so much for your detailed answer, never used promises, it's a good time to learn them! I believe I got the logic and understood in general your code. I just got a question, promiseOfPrices will be an array of all the prices right? In this case, if there was a single reject, I will know because the length of the array 'promiseOfPrices' will be different than the one I expect it to be, is that right? Just to clarify, I am accepting your detailed answer now :)
– TiagoM
Nov 13 '18 at 10:38
@TiagoM Promises are a really great abstraction, and I do suggest reading the docs, since there is more than I can say in one SO post. Answering your question: no.Promise.all
produces a new promise which will resolve when all promises passed to it resolve (success) or reject when any one of them reject (fail). So,promiseOfPrices
will be a Promise that resolves with an array of all prices. It will never resolve with an array of a different length. If any single one fails, thepromiseOfPrices
promise will reject (fail) with that first error.
– Yona Appletree
Nov 13 '18 at 20:39
The google introductory tutorial on Promises seems pretty good: developers.google.com/web/fundamentals/primers/promises
– Yona Appletree
Nov 13 '18 at 20:43
Put another way, if you want to know if anything went wrong, look at the example I gave about doing more work, with two arguments to.then()
or you could use.catch()
to catch errors.
– Yona Appletree
Nov 13 '18 at 20:47
Hello there @YonaAppletree just wanted to pass by and say thank you for the docs and explanation! I got it working finally the way I need, using promises.All and Then clause, also I will read the doc you sent me because it sounds very well documented! Thank you again for all the help and your time! All the best ;)
– TiagoM
Nov 14 '18 at 1:19
add a comment |
Hi @YonaAppletree thank you so much for your detailed answer, never used promises, it's a good time to learn them! I believe I got the logic and understood in general your code. I just got a question, promiseOfPrices will be an array of all the prices right? In this case, if there was a single reject, I will know because the length of the array 'promiseOfPrices' will be different than the one I expect it to be, is that right? Just to clarify, I am accepting your detailed answer now :)
– TiagoM
Nov 13 '18 at 10:38
@TiagoM Promises are a really great abstraction, and I do suggest reading the docs, since there is more than I can say in one SO post. Answering your question: no.Promise.all
produces a new promise which will resolve when all promises passed to it resolve (success) or reject when any one of them reject (fail). So,promiseOfPrices
will be a Promise that resolves with an array of all prices. It will never resolve with an array of a different length. If any single one fails, thepromiseOfPrices
promise will reject (fail) with that first error.
– Yona Appletree
Nov 13 '18 at 20:39
The google introductory tutorial on Promises seems pretty good: developers.google.com/web/fundamentals/primers/promises
– Yona Appletree
Nov 13 '18 at 20:43
Put another way, if you want to know if anything went wrong, look at the example I gave about doing more work, with two arguments to.then()
or you could use.catch()
to catch errors.
– Yona Appletree
Nov 13 '18 at 20:47
Hello there @YonaAppletree just wanted to pass by and say thank you for the docs and explanation! I got it working finally the way I need, using promises.All and Then clause, also I will read the doc you sent me because it sounds very well documented! Thank you again for all the help and your time! All the best ;)
– TiagoM
Nov 14 '18 at 1:19
Hi @YonaAppletree thank you so much for your detailed answer, never used promises, it's a good time to learn them! I believe I got the logic and understood in general your code. I just got a question, promiseOfPrices will be an array of all the prices right? In this case, if there was a single reject, I will know because the length of the array 'promiseOfPrices' will be different than the one I expect it to be, is that right? Just to clarify, I am accepting your detailed answer now :)
– TiagoM
Nov 13 '18 at 10:38
Hi @YonaAppletree thank you so much for your detailed answer, never used promises, it's a good time to learn them! I believe I got the logic and understood in general your code. I just got a question, promiseOfPrices will be an array of all the prices right? In this case, if there was a single reject, I will know because the length of the array 'promiseOfPrices' will be different than the one I expect it to be, is that right? Just to clarify, I am accepting your detailed answer now :)
– TiagoM
Nov 13 '18 at 10:38
@TiagoM Promises are a really great abstraction, and I do suggest reading the docs, since there is more than I can say in one SO post. Answering your question: no.
Promise.all
produces a new promise which will resolve when all promises passed to it resolve (success) or reject when any one of them reject (fail). So, promiseOfPrices
will be a Promise that resolves with an array of all prices. It will never resolve with an array of a different length. If any single one fails, the promiseOfPrices
promise will reject (fail) with that first error.– Yona Appletree
Nov 13 '18 at 20:39
@TiagoM Promises are a really great abstraction, and I do suggest reading the docs, since there is more than I can say in one SO post. Answering your question: no.
Promise.all
produces a new promise which will resolve when all promises passed to it resolve (success) or reject when any one of them reject (fail). So, promiseOfPrices
will be a Promise that resolves with an array of all prices. It will never resolve with an array of a different length. If any single one fails, the promiseOfPrices
promise will reject (fail) with that first error.– Yona Appletree
Nov 13 '18 at 20:39
The google introductory tutorial on Promises seems pretty good: developers.google.com/web/fundamentals/primers/promises
– Yona Appletree
Nov 13 '18 at 20:43
The google introductory tutorial on Promises seems pretty good: developers.google.com/web/fundamentals/primers/promises
– Yona Appletree
Nov 13 '18 at 20:43
Put another way, if you want to know if anything went wrong, look at the example I gave about doing more work, with two arguments to
.then()
or you could use .catch()
to catch errors.– Yona Appletree
Nov 13 '18 at 20:47
Put another way, if you want to know if anything went wrong, look at the example I gave about doing more work, with two arguments to
.then()
or you could use .catch()
to catch errors.– Yona Appletree
Nov 13 '18 at 20:47
Hello there @YonaAppletree just wanted to pass by and say thank you for the docs and explanation! I got it working finally the way I need, using promises.All and Then clause, also I will read the doc you sent me because it sounds very well documented! Thank you again for all the help and your time! All the best ;)
– TiagoM
Nov 14 '18 at 1:19
Hello there @YonaAppletree just wanted to pass by and say thank you for the docs and explanation! I got it working finally the way I need, using promises.All and Then clause, also I will read the doc you sent me because it sounds very well documented! Thank you again for all the help and your time! All the best ;)
– TiagoM
Nov 14 '18 at 1:19
add a comment |
This is how you can read the items in sequence using promises (async/await flavour):
var orderItems = userData.shoppingcart;
let totalPrice = 0;
for (let itemName of userData.shoppingcart)
const itemData = await _data.read('menuitems', itemName);
totalPrice += itemData.price;
This example assumes that _data.read
supports async/await
. If it doesn't, however, it can "promisified" using the promisify
function in nodejs' util module
…assuming that_data.read
supports promises.
– Bergi
Nov 11 '18 at 21:55
Yeah, and this runs in sequence, rather than parallel, which isn't likely desirable. One of my biggest issues withasync
/await
is that it seems to encourage less parallel code.
– Yona Appletree
Nov 12 '18 at 19:31
@bergi yes,async/await
is assumed in this case, will add it to the answer.
– Christian
Nov 12 '18 at 19:36
@YonaAppletree I find it more often than not that it's desirable to optimise for readability rather than performance.
– Christian
Nov 12 '18 at 19:39
1
@Christian Of course. But there's performance and there's performance. In this case, you're taking something that might be O(1) and turning it into O(N). That's just not the same as something like the intermediate arrays created when using comprehensions. O(1) vs O(N) is huge, especially if there's network or file reads or whatever. It's at least worth considering carefully.
– Yona Appletree
Nov 13 '18 at 2:06
add a comment |
This is how you can read the items in sequence using promises (async/await flavour):
var orderItems = userData.shoppingcart;
let totalPrice = 0;
for (let itemName of userData.shoppingcart)
const itemData = await _data.read('menuitems', itemName);
totalPrice += itemData.price;
This example assumes that _data.read
supports async/await
. If it doesn't, however, it can "promisified" using the promisify
function in nodejs' util module
…assuming that_data.read
supports promises.
– Bergi
Nov 11 '18 at 21:55
Yeah, and this runs in sequence, rather than parallel, which isn't likely desirable. One of my biggest issues withasync
/await
is that it seems to encourage less parallel code.
– Yona Appletree
Nov 12 '18 at 19:31
@bergi yes,async/await
is assumed in this case, will add it to the answer.
– Christian
Nov 12 '18 at 19:36
@YonaAppletree I find it more often than not that it's desirable to optimise for readability rather than performance.
– Christian
Nov 12 '18 at 19:39
1
@Christian Of course. But there's performance and there's performance. In this case, you're taking something that might be O(1) and turning it into O(N). That's just not the same as something like the intermediate arrays created when using comprehensions. O(1) vs O(N) is huge, especially if there's network or file reads or whatever. It's at least worth considering carefully.
– Yona Appletree
Nov 13 '18 at 2:06
add a comment |
This is how you can read the items in sequence using promises (async/await flavour):
var orderItems = userData.shoppingcart;
let totalPrice = 0;
for (let itemName of userData.shoppingcart)
const itemData = await _data.read('menuitems', itemName);
totalPrice += itemData.price;
This example assumes that _data.read
supports async/await
. If it doesn't, however, it can "promisified" using the promisify
function in nodejs' util module
This is how you can read the items in sequence using promises (async/await flavour):
var orderItems = userData.shoppingcart;
let totalPrice = 0;
for (let itemName of userData.shoppingcart)
const itemData = await _data.read('menuitems', itemName);
totalPrice += itemData.price;
This example assumes that _data.read
supports async/await
. If it doesn't, however, it can "promisified" using the promisify
function in nodejs' util module
edited Nov 12 '18 at 19:45
answered Nov 11 '18 at 21:09
Christian
3,00622544
3,00622544
…assuming that_data.read
supports promises.
– Bergi
Nov 11 '18 at 21:55
Yeah, and this runs in sequence, rather than parallel, which isn't likely desirable. One of my biggest issues withasync
/await
is that it seems to encourage less parallel code.
– Yona Appletree
Nov 12 '18 at 19:31
@bergi yes,async/await
is assumed in this case, will add it to the answer.
– Christian
Nov 12 '18 at 19:36
@YonaAppletree I find it more often than not that it's desirable to optimise for readability rather than performance.
– Christian
Nov 12 '18 at 19:39
1
@Christian Of course. But there's performance and there's performance. In this case, you're taking something that might be O(1) and turning it into O(N). That's just not the same as something like the intermediate arrays created when using comprehensions. O(1) vs O(N) is huge, especially if there's network or file reads or whatever. It's at least worth considering carefully.
– Yona Appletree
Nov 13 '18 at 2:06
add a comment |
…assuming that_data.read
supports promises.
– Bergi
Nov 11 '18 at 21:55
Yeah, and this runs in sequence, rather than parallel, which isn't likely desirable. One of my biggest issues withasync
/await
is that it seems to encourage less parallel code.
– Yona Appletree
Nov 12 '18 at 19:31
@bergi yes,async/await
is assumed in this case, will add it to the answer.
– Christian
Nov 12 '18 at 19:36
@YonaAppletree I find it more often than not that it's desirable to optimise for readability rather than performance.
– Christian
Nov 12 '18 at 19:39
1
@Christian Of course. But there's performance and there's performance. In this case, you're taking something that might be O(1) and turning it into O(N). That's just not the same as something like the intermediate arrays created when using comprehensions. O(1) vs O(N) is huge, especially if there's network or file reads or whatever. It's at least worth considering carefully.
– Yona Appletree
Nov 13 '18 at 2:06
…assuming that
_data.read
supports promises.– Bergi
Nov 11 '18 at 21:55
…assuming that
_data.read
supports promises.– Bergi
Nov 11 '18 at 21:55
Yeah, and this runs in sequence, rather than parallel, which isn't likely desirable. One of my biggest issues with
async
/await
is that it seems to encourage less parallel code.– Yona Appletree
Nov 12 '18 at 19:31
Yeah, and this runs in sequence, rather than parallel, which isn't likely desirable. One of my biggest issues with
async
/await
is that it seems to encourage less parallel code.– Yona Appletree
Nov 12 '18 at 19:31
@bergi yes,
async/await
is assumed in this case, will add it to the answer.– Christian
Nov 12 '18 at 19:36
@bergi yes,
async/await
is assumed in this case, will add it to the answer.– Christian
Nov 12 '18 at 19:36
@YonaAppletree I find it more often than not that it's desirable to optimise for readability rather than performance.
– Christian
Nov 12 '18 at 19:39
@YonaAppletree I find it more often than not that it's desirable to optimise for readability rather than performance.
– Christian
Nov 12 '18 at 19:39
1
1
@Christian Of course. But there's performance and there's performance. In this case, you're taking something that might be O(1) and turning it into O(N). That's just not the same as something like the intermediate arrays created when using comprehensions. O(1) vs O(N) is huge, especially if there's network or file reads or whatever. It's at least worth considering carefully.
– Yona Appletree
Nov 13 '18 at 2:06
@Christian Of course. But there's performance and there's performance. In this case, you're taking something that might be O(1) and turning it into O(N). That's just not the same as something like the intermediate arrays created when using comprehensions. O(1) vs O(N) is huge, especially if there's network or file reads or whatever. It's at least worth considering carefully.
– Yona Appletree
Nov 13 '18 at 2:06
add a comment |
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53252200%2fhow-to-optimize-this-block-of-code-since-its-async%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
"what if both files are read at same time" - isn't that exactly what you want?
– Bergi
Nov 11 '18 at 19:23
1
Basically, yes that's the idea - counting how many of the tasks are done, and then continue. There are many pitfalls though, like taking errors into account correctly or the edge case of 0 iterations (like on an empty input), so you better use a some proven existing code instead of reinventing the wheel. Learn about promises and
Promise.all
.– Bergi
Nov 11 '18 at 19:25
Yes they can all read at the same time, but I want the flux to continue ONLY when all have been read, so I have the right and exact amount of totalPrice variable. Yes I have heard about Promise, but don't know how to use it, will do some search on this, thanks @Bergi
– TiagoM
Nov 11 '18 at 19:34