Ken Ingram
Ken Ingram

Reputation: 1598

Am I using this Promise functionality properly

I'm using node to grab some data from a url using cheerio.

const request=require('request');
const cheerio=require('cheerio');
const Promise = require('promise');

The function getDataParms(parm1, parm2) returns a promise object.

getDataParms is called for a set of changing parameters by retrieveAllData(parm1, limit)

The final output comes from

var test2 = retrieveAllData('foo','2015');
console.log(test2);

The output of node script.js

// [ Promise { _75: 0, _83: 0, _18: null, _38: null } ]

Somewhere I'm not using the promise methodology correctly, and I can't tell where. I needs some experienced eyes to help me figure out what I'm doing wrong.

The code:

const request=require('request');
const cheerio=require('cheerio');
const Promise = require('promise');

var dateVal = new Date();
var test2 = [];

function retrieveAllData(parm1, limit){
    var output = [];
    var intermediate;

    for (var j=1; j <= limit; j++){
        var objKey = parm1 + "_data";
        var results = {
            "data1": null,
            [objKey]: null
        };

        results.data1 = j;
        objKey = objKey + "_" + j;
        results[objKey] = getDataParms(parm1, j).then(function(value){
            //console.log(value);
            return value;
        });

        //console.log(results[objKey]);
        output.push(results[objKey]);
    }
    return output;
}

// Returns a Promise array
function getDataParms(parm1, parm2){
    var sourceURL = "http://website/source=" + material + "&parm1=" + parm1 + "&parm2=parm2";
    var parsedResults = [];
    var metadata = {
      record_parm2: time_period,
      record_no: null,
      record_date: null,
      col1: null,
      col2: null,
      col3: null
    };

    return new Promise(function(fulfill, reject){
            request(sourceURL, function(error,response,html){
              if (error){
                reject(error);
              } else if (!error && response.statusCode == 200){
                var $ = cheerio.load(html);
                $(".data tr").each(function(i, element){
                    metadata.record_no = i;
                        $(this).find("td").each(function(cellindex){
                          switch(cellindex){
                           case 0:
                                metadata.record_date = $(this).text();
                            break;
                           case 1:
                                metadata.col1 = parseFloat($(this).text());
                            break;
                           case 2:
                                metadata.col2 = parseFloat($(this).text());
                            break;
                           case 3:
                                metadata.col3 = parseFloat($(this).text());
                            break;
                          }
                      });   

                    parsedResults.push(metadata);
                });

                fulfill(parsedResults);
                }
        });
    });
}

var test2 = retrieveAllData('foo','2015');
console.log(test2);

Upvotes: 0

Views: 63

Answers (2)

CertainPerformance
CertainPerformance

Reputation: 370619

Because each getDataParms call returns a Promise, you should wait for all such Promises to resolve first with Promise.all. Also, because getDataParms returns a Promise, retrieveAllData, which consumes getDataParams, should also return a Promise in order for the eventual results to be usable later. Instead of var test2 = retrieveAllData(..., you should call .then on the retrieveAllData call.

function retrieveAllData(parm1, limit){
  // Create an array of Promises, with `j`'s values being 0 to `limit - 1`:
  const allPromises = Array.from(
    { length: limit },
    (_, j) => {
      // Make j start at 1 rather than 0:
      j++;
      const objKey = parm1 + "_data_" + j;
      // After getDataParms resolves, return an object with keys `data1` and `[objKey]`:
      return getDataParms(parm1, j)
        .then((parms) => ({
          data1: j,
          [objKey]: parms
        }));
    }
  );
  return Promise.all(allPromises);
}

And consume it with .then:

retrieveAllData('foo','2015')
  .then(test2 => {
    console.log(test2);
  });

Using a for loop rather than being functional would look like this:

function retrieveAllData(parm1, limit){
  const allPromises = [];
  for (let year = 1990; year <= limit; year++) {
    const objKey = parm1 + "_data_" + year;
    allPromises.push(
      getDataParms(parm1, j)
      .then((parms) => ({
        data1: j,
        [objKey]: parms
      }))
    )
  }
  return Promise.all(allPromises);
}

Upvotes: 2

Arif Khan
Arif Khan

Reputation: 5069

You need to use Promise.all in your case, following example may help you

function retrieveAllData(parm1, limit) {
    var output = [];
    for (var j = 1; j <= limit; j++) {
        output.push(getDataParms(parm1, j));
    }
    return Promise.all(output);
}

var test2 = retrieveAllData('foo', '2015'); // this will return a promise
test2.then(function (result) {
    console.log(result);
})

Upvotes: 0

Related Questions