0

I have a function which makes 1 API call returning a Promise. Whats the best way to convert this code to make n calls for different timestamps. I would like to return an object with the unaggregated data stored by key timestamp.

This code runs for i = 1 and histData is undefined.

const getEthPriceHistorical= (toSymbol) => {
    var histData = {};
    var ts;
    for (let i = 1; i < 5; i++){
        ts =  new Date(new Date().getTime() - (24*i * 60 * 60 * 1000));

        if (typeof toSymbol === 'string') {
           toSymbol = toSymbol.toUpperCase();
        } else {
            toSymbol = 'USD,EUR,GBP,CHF,THB,AUD,INR';
        }
        ***** WHAT TO DO WITH THIS SINGLE PROMISE *****
        return popsicle.request({
           method: 'POST',
           url: 'https://min-api.cryptocompare.com/data/pricehistorical',

           query: {
            fsym: 'ETH',
            tsyms: toSymbol,
            timestamp: ts
           }
       })
         .use(popsicle.plugins.parse(['json']))
         .then(resp => resp.body)
         .then(data => {
            const symbols = Object.keys(data);
            histData.ts = data;
            console.log(ts, data);
        });
  }
  return histData;
}
codervince
  • 316
  • 4
  • 15
  • you know a return in a for loop will return out of the enclosing function, so the loop will never run more than one iteration, right – Jaromanda X Jun 12 '17 at 01:16
  • so, `return histData` will never be executed either, but that doesn't matter because, even if you did get to that return, the asynchronous nature of the code would ensure that histData would never be populated before it was returned. - you need to understand https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call (as well as figure out what to do with the loop) – Jaromanda X Jun 12 '17 at 01:23
  • thirdly, once you've figured all that out, this line `histData.ts = data` will overwrite the same property all the time - perhaps it's meant to be `histData[ts] = data` – Jaromanda X Jun 12 '17 at 01:25
  • The problem with my initial version was that I was trying to mix a single call with multiple calls with Promises. Promise.all is what I was looking for. – codervince Jun 15 '17 at 04:04

2 Answers2

2

The issues with your code are:

  1. the return in the loop, returns from the function - which is why the loop runs only once
  2. histData a variable local to that function, is populated asynchronously. Not visible outside that function, and I can't see inside that function where you even determine that it's undefined
  3. the line histData.ts = data would change the ts property in each iteration (if there were more than one) - should be histData[ts] = data.

Hopefully the code below is self explanatory once you take into consideration the errors in your code

note: Array.from({length: 4}) creates an array with four undefined entries in index 0...3 - hence why the timestamp uses (i+1)

const getEthPriceHistorical= (toSymbol) => {
    if (typeof toSymbol === 'string') {
       toSymbol = toSymbol.toUpperCase();
    } else {
        toSymbol = 'USD,EUR,GBP,CHF,THB,AUD,INR';
    }
    return Promise.all(Array.from({length:4}).map((unused, i) => {
        let ts =  new Date(new Date().getTime() - (24*(i+1) * 60 * 60 * 1000));
        return popsicle.request({
           method: 'POST',
           url: 'https://min-api.cryptocompare.com/data/pricehistorical',

           query: {
            fsym: 'ETH',
            tsyms: toSymbol,
            timestamp: ts
           }
       })
         .use(popsicle.plugins.parse(['json']))
         .then(resp => resp.body)
         .then(data => {
            const symbols = Object.keys(data);
            return {ts, data};
        });
    })).then(results => results.reduce((result, {ts, data}) => {
        result[ts] = data;
        return result;
    }, {}));
}

alternatively, the last 8 or so lines can be

        .then(data => {
            const symbols = Object.keys(data);
            return {[ts]: data};
        });
    }))
    .then(results => results.reduce((result, item) => Object.assign(result, item), {}));
}

note: either way, what is const symbols = Object.keys(data); besides redundant code that does nothing?

Jaromanda X
  • 53,868
  • 5
  • 73
  • 87
  • thanks. Promise.all was what I was looking for. Correct about Object.keys(data) does nothing and has been removed. I prefer this solution bc it doesnt need the extra nsynjs dependency and the code is being called from outside the module. – codervince Jun 15 '17 at 04:01
0

You can write your logic as if it was synchronous, and run it via synchronous executor nsynjs:

main.js:

var nsynjs = require('nsynjs');
var popsicle = require('popsicle');

var getEthPriceHistorical = function(popsicle, toSymbol) {
    var histData = {};
    var ts;
    for (var i = 1; i < 5; i++){
        ts =  new Date(new Date().getTime() - (24*i * 60 * 60 * 1000));

        if (typeof toSymbol === 'string') {
           toSymbol = toSymbol.toUpperCase();
        } else {
            toSymbol = 'USD,EUR,GBP,CHF,THB,AUD,INR';
        }
        var data = popsicle.request({
           method: 'POST',
           url: 'https://min-api.cryptocompare.com/data/pricehistorical',

           query: {
            fsym: 'ETH',
            tsyms: toSymbol,
            timestamp: ts
           }
       })
         .data.body;

       histData[ts] = JSON.parse(data);
       console.log(ts+", got: "+data);
  }
  return histData;
}

var ctx = nsynjs.run(getEthPriceHistorical,{},popsicle,null,function(histData){
    console.log('done: '+ JSON.stringify(histData));
});

Please run this code here: https://runkit.com/amaksr/node-popsicle

amaksr
  • 7,555
  • 2
  • 16
  • 17
  • I like this approach. I preferred the Promises.all version below because I didn't want to have the extra dependency in the calling code outside the module. Works well otherwise. – codervince Jun 15 '17 at 04:03