0

Hi so i have been trying to make this tried using async module didn't really know how to convert this to one tried promising it didn't really work well i think i did it wrong so i reverted the function to the way it was at first

Basically i want to wait till the ReadJson() function is done with reading all the json files that are in the array then do other functions like editjson etc

Code:

App.js

const Reader = require('./Reader');
Reader.ReadJson();

Reader.js

const fsp = require('fs-promise');
const JsonFiles = ['json1.json', 'json2.json', 'json3.json', 'json4.json'];
const JsonContents = [];
class Reader {
    static ReadJson() {
        JsonFiles.forEach(name => {
            let FileDir = "D:\\Development\\Java\\" + name;
            fsp.readJson(FileDir).then(contents => {
                if (contents) {
                    JsonContents.push(contents);
                    console.log(`Loaded >> ${name} ${Reader.JsonContents.length}/${JsonFiles.length}`);
                }
            });
        });
        console.log('Done Reading Json Content!');
        //Other functions
    }
}
Reader.JsonContents = JsonContents;
module.exports = Reader;

So basically the output is:

Done Reading Json Content!
Loaded >> json1.json 1/4
Loaded >> json2.json 2/4
Loaded >> json3.json 3/4
Loaded >> json4.json 4/4

When i need it to be:

Loaded >> json1.json 1/4
Loaded >> json2.json 2/4
Loaded >> json3.json 3/4
Loaded >> json4.json 4/4
Done Reading Json Content!

Thank you :)

Premt
  • 71
  • 1
  • 1
  • 8
  • 1
    http://stackoverflow.com/questions/18983138/callback-after-all-asynchronous-foreach-callbacks-are-completed ? – Sergio Alen Feb 13 '17 at 07:04
  • 1
    There are dozens of questions with more than dozens of answers here already on stackoverflow about how to sequence a loop of async operations like you have. I'd suggest you find those and learn from them. – jfriend00 Feb 13 '17 at 07:11
  • 1
    One solution: [Only continue for loop when async get request finishes](http://stackoverflow.com/questions/33662425/only-continue-for-loop-when-async-get-request-finishes/33662492#33662492) – jfriend00 Feb 13 '17 at 07:13

2 Answers2

4

Return a promise, track your progress in the forEach and resolve it only when JsonContents length is the same as JsonFiles length.

const fsp = require('fs-promise');
const JsonFiles = ['json1.json', 'json2.json', 'json3.json', 'json4.json'];
const JsonContents = [];
class Reader {
    static ReadJson() {
        return new Promise((resolve, reject) => {
            JsonFiles.forEach(name => {
                let FileDir = "D:\\Development\\Java\\" + name;
                fsp.readJson(FileDir).then(contents => {
                    if (contents) {
                        JsonContents.push(contents);
                        console.log(`Loaded >> ${name} ${Reader.JsonContents.length}/${JsonFiles.length}`);
                    }
                    if (JsonContents.length == JsonFile.length) {
                        return resolve(JsonContents);
                    }
                }).catch(err => {
                    return reject(err);
                });
            });
        });
    }
}
Reader.JsonContents = JsonContents;
module.exports = Reader;

And then use it in your app:

const Reader = require('./Reader');
Reader.ReadJson().then(() => { console.log('Done Reading Json Content!'); });

Another option is using Promise.all, because you are using fs-promise, but although it can be done with forEach, a regular for loop is better here.

const fsp = require('fs-promise');
const JsonFiles = ['json1.json', 'json2.json', 'json3.json', 'json4.json'];
const JsonContents = [];
class Reader {
    static ReadJson() {
        var promises = [];
        for (let i = 0; i < JsonFiles.length; i++) {
            let FileDir = "D:\\Development\\Java\\" + JsonFiles[i];
            promises.push(fsp.readJson(FileDir).then(contents => {
                if (contents) {
                    JsonContents.push(contents);
                    console.log(`Loaded >> ${JsonFiles[i]} ${Reader.JsonContents.length}/${JsonFiles.length}`);
                }
            }));

        }
        return Promise.all(promises);
    }
}
Reader.JsonContents = JsonContents;
module.exports = Reader;
Ron Dadon
  • 2,666
  • 1
  • 13
  • 27
  • This is an anti-pattern to create a wrapper promise when `fsp.readJson()` already makes promises. You can just chain those promises if you want sequential operation. – jfriend00 Feb 13 '17 at 07:19
  • @jfriend00 That is the purpose of `Promise.all` example, and also there is a recursive promise option that I did not mentioned so it won't be "over complicated" solution. – Ron Dadon Feb 13 '17 at 07:22
  • Do you understand what the [promise anti-patterns are](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns)? You can sequence your first code block and still not wrap in a newly created promise by using the promise returned form `fsp.readJson()` more intelligently. Doing so would also have other benefits, like it would stop looping when it got an error rather than continue through the whole rest of the loop like your solution does. – jfriend00 Feb 13 '17 at 07:53
  • I accept your comment, but can not see how you can do it without recursion. If it can be done, I would love to see and learn how – Ron Dadon Feb 13 '17 at 12:38
1

As an addendum to Ron Dadon's Promise.all method....

The Bluebird promise library provides some helper functions like Promise.map and Promise.filter that can remove a lot of the boiler plate of Promise array processing code.

const Promise = require('bluebird');
const fsp = require('fs-promise');
const path = require('path');


class Reader {

  static readFiles(jsonPath, jsonFiles){
    let fileReadCount = 0;
    return Promise.map(jsonFiles, name => {
        let filePath = path.join(jsonPath, name);
        return fsp.readJson(filePath);
      })
      .filter((content, index, length) => {
        if (!content) return false;
        console.log(`Loaded >> ${jsonFiles[index]} ${index+1} / ${length}`);
        return true;
      })
  }

  static readJson() {
    return this.readFiles(this.jsonPath, this.jsonFiles).then(contents => {
      console.log('Done Reading Json Content!', contents);
      return this.jsonContents = contents;
    })
  }

}

Reader.jsonFiles = ['json1.json', 'json2.json', 'json3.json', 'json4.json'];
Reader.jsonPath = 'D:\\Development\\Java';

module.exports = Reader;
Matt
  • 68,711
  • 7
  • 155
  • 158