What the code does (tested basic, no errors, usecase):

  • Reads all XML files (ending in .yin) from a directory.
  • converts each to JSON
  • store it in another directory, with the same basename as source file
  • ... with .yin replaced by .json

Overt issues:

  • The code has too many nested functions
  • walk module's next() method in multiple places
  • lack of robust error checks and ensuring no uncaught errors left behind

Q. How to refactor the code using Node.js best practices? (things like):

  • flatten out the nesting.
  • avoiding having to remember to call next(), on all if(err) scenarios.
  • does any other (than those included) module improve readability/efficiency?

Two Node.js files, a module and a main file invoking that module, are attached.

File 1: Main:

// main.js
var path = require ('path');
var yin2json = require ('./yin2json');

console.log ("__dirname: " + __dirname);
var templateDir = path.resolve (__dirname, "resources", "mgmtDataTemplates");
var yinDir = path.resolve (templateDir, "yins");
var jsonDir = path.resolve (templateDir, "jsons");

yin2json.convertYins (yinDir, jsonDir);

File 2: Module:

// yin2json.js

    var fs = require('fs');
    var path = require('path');
    var walk = require('walk');
    var xml2js = require('xml2js');

    var jsonDir ;
    var convertYins = function (yin_dir, json_dir) {
        jsonDir =  json_dir;

        var walker = walk.walk(yin_dir, { followLinks: true });
        walker.on("errors", fDirWalkError);
        walker.on("end", fDirWalkEnd);
        walker.on("file", fDirWalkFile);


    function fDirWalkError (err) {
        console.log ("fDirWalkError: " + err);
        next (err);

    function fDirWalkEnd  () {
        console.log ("======= End of directory walk");

    function fDirWalkFile (root, fileStat, next) {
        if (fileStat.name.indexOf(".yin") < 0) {
            console.log ("skipping file " + fileStat.name + " (does not end in .yin)");
        } else {
            var yin_file = path.resolve(root, fileStat.name);
            console.log ("yin file: " + yin_file);

            fs.readFile(yin_file, function (err, data) {
                if (err) {
                    console.log ("error reading file:" + yin_file);
                    next (err);
                xml2js.parseString (data, function (err, json_obj) {
                    if (err) {
                        console.log (err);
                        next (err);
                    var json_string = JSON.stringify(json_obj, null, 2);
                    var json_file = path.resolve (jsonDir, path.basename(yin_file).replace(/\.yin$/, ".json"));
                    console.log ("json file: ", json_file);

                    fs.writeFile(json_file, json_string, "utf8", function (err) {
                        if (err) {
                            console.log ("error converting yin (%s) to json(%s)", yin_file, json_file);
                            next (new Error ("error converting yin(" + yin_file + ") to json(" + json_file + ")"));
                        else {
                            console.log ("Converted yin (%s) to json(%s)", yin_file, json_file);
        next ();

module.exports.convertYins = convertYins;

1 Answer 1


Try using a Promise library like bluebird. it will save you from callback hell and allow you to write your code synchronously.

let Promise = require('bluebird'),
    fs = Promise.promisifyAll(require('fs'))

function readAllFilesInDir(pathArr) {
    // assume i get all files path as array here 
    let allPromises = pathArr.forEach(function (path) {
        return fs.readFileAsync(path, 'utf8')

    return allPromises

function doYourThing() {
    // body...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.