0

The Node.js function below takes:

  • an object, shop which contains a regular expression
  • an array of filenames

The function will read each csv file listed in the array and test a cell in the first row with a regular expression, returning a new array of matching filenames.

function matchReport(shop, arr) {

    return promise = new Promise(resolve => {
        var newArray = [];
        for(var j=0;j<arr.length;++j) {
            let filename = arr[j];
            csv()
            .fromFile(filename)
            .then(reportData => {
                if (reportData[0]['Work'].match(shop.productRegex)) {
                    newArray.push(filename);
                }
                if (j === arr.length) {
                    resolve(newArray);
                }
            });
        }
    }).then(matches => {
        return {
            'shop' : shop.name,
            'reports' : matches
        }
    }).catch(e => {
        console.log(e);
    });
}

Very rarely the function will return with the correct behavior which is this:

{ shop: 'shop1',
  reports: [ '../artist-sales-report-2020-11-12(1).csv' ] }
{ shop: 'shop2',
  reports:
   [ '../artist-sales-report-2020-12-03.csv',
     '../artist-sales-report-2020-09-01.csv' ] }

More often it returns with reports missing, like below:

{ shop: 'shop1',
  reports: [ '../artist-sales-report-2020-11-12(1).csv' ] }
{ shop: 'shop2',
  reports: [ '../artist-sales-report-2020-12-03.csv' ] }

I understand where the problem is taking place, within the csv reportData block. I understand that it is an asynchronous issue and I have tried to write more elaborate if..then or switch statements as a hack solution with no luck. It seems a little sloppy and cluttered to me to create more promises inside of this promise but I have been unsuccessful at that as well.

Max McCarty
  • 5
  • 3
  • 1
    This is what `Promise.all` is for - just map all your data to promises that resolve, and then await batch completion. Promises nested in Promises is the whole point - returning promises to promises allows for internal extension of promises, so you could fetch any more data later. Just make sure every promise resolves correctly, then await them all. " It seems a little sloppy and cluttered to me to create more promises inside of this promise" - It isn't, but you should consider using `async/await` to prevent cluttered code. – somethinghere Dec 03 '20 at 21:01

1 Answers1

0

Using async/await and your disliked nested promises you could simplify your code to something like this, which should always await all results. I made the assumption that your problem is the fromFile method, which feels like it is itself asynchronous since it uses a then that you are not awaiting.

async function matchReport(shop, arr) {
    
    const matches = await Promise.all(arr.map(async filename => {
       
        const reportData = await csv().fromFile( filename );

        if( reportData[0]['Work'].match(shop.productRegex) ){
        
            return filename;
            
        }
        
    }));
    
    return {
        'shop': shop.name,
        'reports': matches.filter( Boolean )
    };
    
}
somethinghere
  • 16,311
  • 2
  • 28
  • 42
  • Thanks! I corrected an error (add parentheses to end of `csv`) and added a filter to prevent the returned array from containing any undefined – Max McCarty Dec 03 '20 at 21:15
  • ```async function matchReport(shop, arr) { const matches = await Promise.all(arr.map(async filename => { const reportData = await csv().fromFile( filename ); if (reportData[0]['Work'].match(shop.productRegex)) { return filename; } })); return { 'shop': shop.name, 'reports': matches.filter(function(x) { return x !== undefined; }) }; } ``` – Max McCarty Dec 03 '20 at 21:15
  • Ah yes, the filter is useful. You don't even need a check: `matches.filter( Boolean )` will work as well. I'm not sure what you meant with the parenthesis to end of csv? – somethinghere Dec 03 '20 at 21:27
  • const reportData = await csv().fromFile( filename ); – Max McCarty Dec 03 '20 at 22:03