Search code examples
javascriptnode.jspromisees6-promise

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);

Solution

  • 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);
    }