0
#!/usr/bin/env node

var fs = require('fs')
  , async = require('async') 
  , program = require('commander')

program
  .version('0.0.1')
  .usage('<keywords>')
  .parse(process.argv)

async.waterfall([
  fs.readdir.bind(fs, __dirname),
  parseHTML,
], saveResult)

function parseHTML(files, callback) {
  var result = []

  files.forEach(function(file) {
    if (file.match(/\.html$/)) {
      fs.readFile(file, 'utf8', function(err, data) {
        if (err) throw err
        result.push(data)
      })
    }
  })

  callback(null, result)
}

function saveResult(err, result) {
  console.log(result)
}

I'm confused because console.log(data) does output the data:

<p>File 1</p>

<p>File 2</p>

Yet the final result is an empty array: []

Why am I doing wrong?

alexchenco
  • 53,565
  • 76
  • 241
  • 413
  • http://stackoverflow.com/questions/14220321/how-to-return-the-response-from-an-asynchronous-call – georg Mar 14 '15 at 16:49

1 Answers1

2

You have to wait to look at the result until the last fs.readFile() operation has finished. These are async operations and they complete some time in the future. You are examining the result before any of them have finished.

There are many ways to approach solving this, but this method would likely cause the least change to your code as it just keeps a counter of how many are done:

function parseHTML(files, callback) {
    var result = [],
        cntr = 0;

    files.forEach(function(file) {
        if (file.match(/\.html$/)) {
            fs.readFile(file, 'utf8', function(err, data) {
                if (err) throw err
                result.push(data)
                    // see if we're done processing all the results
                    ++cntr;
                if (cntr === files.length) {
                    callback(null, result);
                }
            });
        } else {
            ++cntr;
            if (cntr === files.length) {
                callback(null, result);
            }
        }
    });
}

I'd personally prefer to use promises and Promise.all() to solve this.

Here's a version using the Bluebird promise library that retains some of your other structure:

var Promise = require("bluebird");
var fs = Promise.promisifyAll(require('fs'));

// your other code here

function parseHTML(files, callback) {
    var promises = [];

    files.forEach(function(file) {
        if (file.match(/\.html$/)) {
            promises.push(fs.readFileAsync(file, 'utf8'));
    });
    Promise.all(promises).then(function(results) {
        // all results in results array
        callback(null, results);
    }, function(err) {
       // error here
    });
}

And, here's a fully promise version:

var Promise = require("bluebird");
var fs = Promise.promisifyAll(require('fs'));

function parseHTML(files) {
    var promises = [];

    files.forEach(function(file) {
        if (file.match(/\.html$/)) {
            promises.push(fs.readFileAsync(file, 'utf8'));
    });
    return Promise.all(promises);
}

fs.readdirAsync(__dirname).then(parseHTML).then(function(results) {
    // files are in the results array here
}).catch(function(err) {
    // error here
});
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Hey, where's the code I was studying it, ha. – alexchenco Mar 14 '15 at 17:02
  • @alexchenco - it's back. I realize it was wrong so temporarily rolled it back. – jfriend00 Mar 14 '15 at 17:04
  • Thanks! Wow, I didn't imagine it was so complicated to make the code wait for a certain task. – alexchenco Mar 14 '15 at 17:06
  • @alexchenco - if you're going to program in node.js, you're going to need to get very familiar with asynchronous operations that finishe sometime later. node.js does not wait for asynchronous operations. The main line of code continues executing and the asynchronous operations finish some time later. If you want to coordinate some action based on the completion of asynchronous operations, you have to write special code to do that. You will probably want to learn promises because they provide a lot of tools for this. The async library you show you are using also provides tools for this. – jfriend00 Mar 14 '15 at 17:10
  • OK, thanks. I'll check the Promises. – alexchenco Mar 14 '15 at 17:10
  • 1
    @alexchenco - I added a promises version of code to my answer. – jfriend00 Mar 14 '15 at 17:14
  • Weird, I get `promises.push(fs.readFileAsync(file, 'utf8'))` – alexchenco Mar 14 '15 at 17:26
  • @alexchenco - there was a typo. `var promises;` needs to be `var promises = [];`. – jfriend00 Mar 14 '15 at 17:33