Outer variables not being altered inside Promise.then function

Question

On node while using thinky.js, I am trying to iterate through a loop and add each item to an array. This however, for some reason is not working.

In another place, it is indentical and working, just without a Promise.then function. Why is this not working?

var fixedItems = [];
for (i in tradeItems) {
  var item = tradeItems[i];
  Item.get(item["id"]).run().then(function(result) {
    var f = { "assetid": result["asset_id"] };
    console.log(f);  // WOrks
    fixedItems.push(f); // Doesn't work
  });
}

console.log(fixedItems); // Nothing

if `Item.get(item["id"]).run()` is asynchronous, then you are logging fixeditems before they have even begun to be executed — Jaromanda X, Oct 18 '15 at 11:14
`In another place, it is indentical and working` - can you show this identical working code? — Jaromanda X, Oct 18 '15 at 11:18

joews · Answer 1 · 2015-10-18T12:44:08.480

2

A Promise represents the future result of a task. In this case you're logging fixedItems before your tasks (the calls to Item.get) have finished working. In other words, the then functions haven't run yet so nothing has been put into fixedItems.

If you want use fixedItems once it contains all of the items, you'll need to wait for all of the promises to resolve.

How you do that depends on the Promise library you're using. This example, with Promise.all, works with many libraries including native ES6 Promises:

// Get an array of Promises, one per item to fetch.
// The Item.get calls start running in parallel immediately.
var promises = Object.keys(tradeItems).map(function(key) {
  var tradeItem = tradeItems[key];
  return Item.get(tradeItem.id);
});

// Wait for all of the promises to resolve. When they do,
//  work on all of the resolved values together.
Promise.all(promises)
  .then(function(results) {
    // At this point all of your promises have resolved.
    // results is an array of all of the resolved values.

    // Create your fixed items and return to make them available
    //  to future Promises in this chain
    return results.map(function(result) {
      return { assetid: result.asset_id }
    });
  })
  .then(function(fixedItems) {
    // In this example, all we want to do is log them
    console.log(fixedItems);
  });

Recommended reading: the HTML5 rocks intro to Promises.

edited Oct 18 '15 at 12:44

answered Oct 18 '15 at 12:28

joews

29,767
10
79
91

This is a good alternative approach to my answer. But I don't think `Promise.all()` executes the promises in parallel (am I mistaken?) If there were a way to execute them in parallel rather than sequentially that could be more efficient. It's a fairly minor point (and probably premature optimization), but I'd be curious to see if there's a simple way to do parallel execution with native promises. – Matt Browne Oct 18 '15 at 12:43
1

You are mistaken - they run in parallel. In fact they are already all running in parallel by the time you call `Promise.all`. A Promise always represents a _running_ task, so if you have a Promise its task is underway. In fact serial execution with Promises needs a little more effort. – joews Oct 18 '15 at 12:44
Ah, I see now (the `map` function is run synchronously, of course, so the promises start executing right away...). What about using `Promise.map`? The article you linked doesn't mention it, but that would work here as well, would it not? (no particular advantage over `Promise.all`, just thought I'd mention it). – Matt Browne Oct 18 '15 at 13:07
You could also use `Promise.map` for libraries that support it. I chose `Promise.all` because it's part of the [ES6 Promise API](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise), which is a lot smaller than many libraries like Bluebird and Q. – joews Oct 19 '15 at 08:37

score 1 · Answer 2 · edited May 23 '17 at 12:29

Your problem is that you are calling console.log(fixedItems) before any of the promises in the loop have finished executing. A better way of doing this that would also solve the asynchronous problem is to put all the item IDs in an array first and retrieve all the items in a single query, which is also more efficient on the database side.

var itemIds = tradeItems.map(function(item) {
    return item.id;
});

var fixedItems = [];

//you would need to write your own getItemsById() function or put the code
//to get the items here
getItemsById(itemIds).then(function(items) {

    items.forEach(function(item) {
        var f = { "assetid": result["asset_id"] };
        fixedItems.push(f);
    });

    whenDone();
});

function whenDone() {
    //you should be able to access fixedItems here
}

I couldn't easily find how to look up multiple records by ID in a single query with thinky, but I did find this page which might help: http://c2journal.com/2013/01/17/rethinkdb-filtering-for-multiple-ids/

While this would be my preferred way of solving this problem, it would also be possible to still use multiple queries and use a promise chain to wait for them all to be resolved before continuing to your subsequent code. If you wanted to go that route, check out this link: http://promise-nuggets.github.io/articles/11-doing-things-in-parallel.html. (Note: I haven't personally used Bluebird, but I think the Bluebird example in that link may be out of date. The map method appears to be the current recommended way to do this with promises: https://stackoverflow.com/a/28167340/560114.)

Update: Or for this latter option, you can just use the code in joews's answer above.

Outer variables not being altered inside Promise.then function

2 Answers2