0

I am polling the price of various Ethereum smart contracts with Web3.js. This is done asynchronously, and many times the values get returned out of order, which means they get pushed to the new array in the wrong order. I want to push values to a new array inside a forEach loop so that I can update values on a page using jQuery. What is the best way to keep things in order?

Steps Taken:

First I splice all of the contracts in order into an array.

var Contracts = [];

Contract.splice(0, 0, EthereumContract.at("0x2a50fb3396455717043ae5aa631d362c94fe13e1"));
Contract.splice(1, 0, EthereumContract.at("0x69ec0658ff334789c7f57edc3a2f28adef1c0ef3"));
Contract.splice(2, 0, EthereumContract.at("0x4d2bab147c32c75c8c9277182e64c69e14cb9f3c"));
Contract.splice(3, 0, EthereumContract.at("0xcd56f2c89128ad71cfd87b0fc78c9e26990b0f66"));

Then I make a new array 'TheCurrentPrice' that should receive the price of each contract.

var TheCurrentPrice = [];

Now I loop through the array in order and apply the Price method to the contracts in the array. I push the returned results into a new array. Unfortunately, it sometimes gets responses out of order and gets pushed incorrectly into the new array.

Contract.forEach(function (Contract) {   
Contract.Price(function (error, result) {

    TheCurrentPrice.push(result);

    });
});

Solution:

Alnitak gave the correct answer, but with incorrect syntax in case anyone tries this in the future.

function PricePromise(Contract) {
return new Promise((resolve, reject) => {

     Contract.Price(function (error, result) {
        if (error) {
            reject(error);
            console.log("Price Error");
        } else {

            resolve(result);
            console.log("Price " + result);

        }
         });
     });
};

Promise.all(Contract.map(PricePromise))
.then(function(Price) {

('#Product-Price-1').attr('data-price', Price[0] / 1000000000000000000);

/*This uses jQuery to update a custom data-* attribute in HTML5. Price[0] is of course the first entry in the array of prices retrieved with the promise.*/

...

});

What I don't quite get is why resolve(result) allows me to forgo a forEach loop of the Price method, but you don't need the forEach, this is the correct syntax.

  • mark the requests with incremental ID and push to that index of the array. ```arr[id] = result``` – Martin Chaov Feb 01 '18 at 08:39
  • 1
    Possible duplicate of [Promise.all: Order of resolved values](https://stackoverflow.com/questions/28066429/promise-all-order-of-resolved-values) – str Feb 01 '18 at 08:39
  • The 'Promise.all: Order of resolved values ' thread I came across before asking. It is apparently part of the answer, but Alnitak answered correctly for the specific use case. The other thread is more abstract, with an answer that is mostly jargon requiring current JS knowledge from after ES6, which is quite recent stuff (three years old). I still say cool and sick. :-) – ThickMiddleManager Feb 01 '18 at 16:43

3 Answers3

2

The .forEach method includes an index, so you could use that to populate TheCurrentPrice:

Contracts.forEach(function (Contract, index) {   
    Contract.Price(function (error, result) {
        TheCurrentPrice[index] = result;
    });
});

Note however that you'll still need some form of synchronisation to tell you when all of the Prices have been obtained.

A better solution might therefore be to wrap the calls to Price as "Promises" and then use Promise.all() to wait for all of them to be resolved.

function PricePromise(contract) {
    return new Promise(function(resolve, reject) {
        contract.Price(function(error, result) {
            if (error) {
                reject(error);
            } else {
                resolve(result);
            }
        });
    });
}

[ NB: Promise libraries such as bluebird.js include functions to automatically "Promisify" functions that match the Web3.js Price function's signature ]

Promise.all(Contracts.map(PricePromise)).then(function(TheCurrentPrice) {
    // continue processing here with the values
    // now already populated in "TheCurrentPrice"
    ...
});

If you end up using bluebird.js the above can be slightly further simplified:

Promise.map(Contracts, PricePromise).then(function(TheCurrentPrice) {
    ...
});

where the Promise.map function (which despite appearances is not a standard function, but one added by Bluebird) includes added functionality that allows you to limit the number of concurrent calls that will be placed.

Alnitak
  • 334,560
  • 70
  • 407
  • 495
  • Thank you for this detailed answer. This is what was missing from other questions and answers I had found. The index part is not enough to prevent a list from being out of order if all values are not returned in time after a timeout. It seems to work if values all come back, but then we are sometimes talking 10 seconds, which is too long for a solution where accurate info is required to be displayed. – ThickMiddleManager Feb 01 '18 at 16:36
  • The index would actually guarantee they're in the right order, but not that they're all present. The `Promise.all` function will return a _rejected_ promise if any of the calls fail, so you can `.catch` those, too. – Alnitak Feb 01 '18 at 23:37
  • I added the solution correcting syntax in my question. I think there are a few ways to write promises, but your idea works well. I couldn't figure out how to put multiple promises in Promise.all(), so I wrote it the long way, which is fine for readability anyway. I used multiple promises to update data-* attributes. – ThickMiddleManager Feb 02 '18 at 04:04
  • @TrumpPaiPence I corrected the syntax, putting in the missing `function` keyword. As for your query about what the `.forEach` isn't necessary, it's replaced by the `.map` call, which takes each `Price` object and passes it to `PricePromise`, returning an array of Promises which is what's then passed to `Promise.all`. – Alnitak Feb 02 '18 at 10:29
0

You can use the second argument of Array.prototype.forEach (the index) :

Contract.forEach(function (Contract, index) {   
    Contract.Price(function (error, result) {

        TheCurrentPrice[index] = result;

    });
});
Paul
  • 139,544
  • 27
  • 275
  • 264
0

You can try to add at forEach's callback a function 'index' parameters, then you can set current price into the right position.

    Contract.forEach(function (Contract, index) {
        Contract.Price(function (error, result) {
            TheCurrentPrice[index]=result;
        });
    });