Search code examples
node.jsasynchronouspromisefsbluebird

Refactor code with promises to read files and convert them to json


I'm trying to do the following: Read the content of a directory to find all the .xml files (I'm using glob but I'd like to use something like fs.readdir from fs), then I want to read every file using fs.readFile and then I want to convert the xml file to JSON objects. I'm using xml2json for this purpose.

Once I have the json objects, I would like to iterate every one of them to get the one property out of it and push it to an array. Eventually, all the code is wrapped in a function that logs the content of the array (once is completed). This code currently works fine but I'm getting to the famous callback hell.

const fs = require('fs');
const glob = require('glob');
const parser = require('xml2json');
let connectors = [] 


function getNames(){
glob(__dirname + '/configs/*.xml', {}, (err, files) => {
     for (let j=0; j < files.length; j++) {
          fs.readFile( files[j], function(err, data) {
             try {
                   let json = parser.toJson(data, {object: true, alternateTextNode:true, sanitize:true})               
                   for (let i=0; i< json.properties.length; i++){
                     connectors.push(json.properties[i].name)
                   if (connectors.length === files.length){return console.log(connectors)}
                   }
              }
              catch(e){
                   console.log(e)
              }
       });
     }
 })
}
getNames()

However, I'd like to move to a more clean and elegant solution (using promises). I've been reading the community and I've found some ideas in some similar posts here or here.

I'd like to have your opinion on how I should proceed for this kind of situations. Should I go for a sync version of readFile instead? Should I use promisifyAll to refactor my code and use promises everywhere? If so, could you please elaborate on what my code should look like?

I've also learned that there's a promises based version of fs from node v10.0.0 onwards. Should I go for that option? If so how should I proceed with the parser.toJson() part. I've also seen that there's another promise-based version called xml-to-json-promise.

I'd really appreciate your insights into this as I'm not very familiar with promises when there are several asynchronous operations and loops involved, so I end up having dirty solutions for situations like this one.

Regards, J


Solution

  • I would indeed suggest that you use the promise-version of glob and fs, and then use async, await and Promise.all to get it all done.

    NB: I don't see the logic about the connectors.length === files.length check, as in theory the number of connectors (properties) can be greater than the number of files. I assume you want to collect all of them, irrespective of their number.

    So here is how the code could look (untested):

    const fs = require('fs').promises; // Promise-version (node 10+)
    const glob = require('glob-promise'); // Promise-version
    const parser = require('xml2json');
    
    async function getNames() {
        let files = await glob(__dirname + '/configs/*.xml');
        let promises = files.map(fileName => fs.readFile(fileName).then(data =>
            parser.toJson(data, {object: true, alternateTextNode:true, sanitize:true})
                  .properties.map(prop => prop.name)
        ));
        return (await Promise.all(promises)).flat();
    }
    
    getNames().then(connectors => {
        // rest of your processing that needs access to connectors...
    });
    

    As in comments you write that you have problems with accessing properties.map, perform some validation, and skip cases where there is no properties:

    const fs = require('fs').promises; // Promise-version (node 10+)
    const glob = require('glob-promise'); // Promise-version
    const parser = require('xml2json');
    
    async function getNames() {
        let files = await glob(__dirname + '/configs/*.xml');
        let promises = files.map(fileName => fs.readFile(fileName).then(data =>
            (parser.toJson(data, {object: true, alternateTextNode:true, sanitize:true})
                  .properties || []).map(prop => prop.name)
        ));
        return (await Promise.all(promises)).flat();
    }
    
    getNames().then(connectors => {
        // rest of your processing that needs access to connectors...
    });