Make forEach asynchronous in JavaScript

Question

I'm trying to understand the asynchronous programming Node.js but stalled on this code.

This function in their callback returns an array of files in a directory:

function openDir(path, callback) {
    path = __dirname + path;
    fs.exists(path, function (exists) {
        if (exists) {
            fs.readdir(path, function (err, files) {
                if (err) {
                    throw err;
                }
                var result = [];
                files.forEach(function (filename, index) {
                    result[index] = filename;
                });
                return callback(result);
            });
        }
    });
}

But when I use asynchronous code inside.forEach, it returns nothing:

function openDir(path, callback) {
    path = __dirname + path;
    fs.exists(path, function (exists) {
        if (exists) {
            fs.readdir(path, function (err, files) {
                if (err) {
                    throw err;
                }
                var result = [];
                files.forEach(function (filename, index) {
                    fs.stat(path + filename, function (err, stats) {
                        if (err) {
                            throw err;
                        }
                        result[index] = filename;
                    });
                });
                return callback(result);
            });
        }
    });
}

I understand why it happens, but don't understand how to write correct code.

You can't return from an asynchronous function, you have to set up callback — Paul S.
– Paul S., Commented Feb 13, 2014 at 13:10
This module will make your life easier: github.com/caolan/async — joao
– joao, Commented Feb 13, 2014 at 13:10
Callbacks or promise/deferred pattern is your way to go here. — dfsq
– dfsq, Commented Feb 13, 2014 at 13:11
@PaulS. There was already a callback in his code, but it was misused. — plalx
– plalx, Commented Feb 13, 2014 at 13:18

plalx · Accepted Answer · 2014-02-13 13:17:03Z

5

The issue is that fs.stat is also async, but you could probably do something like:

var result = [],
    expectedLoadCount = files.length,
    loadCount = 0;

files.forEach(function (filename, index) {
    fs.stat(path + filename, function (err, stats) {
        if (err) {
            throw err;
        }
        result[index] = filename;
        if (++loadCount === expectedLoadCount) callback(result);
    });
});

answered Feb 13, 2014 at 13:17

plalx

43.8k7 gold badges77 silver badges94 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

noseratio · Accepted Answer · 2014-02-14 06:21:15Z

2

The other answers may work well, but they are currently quite different semantically from the original code: they both execute stats in parallel, rather than sequentially. The forEach will initiate as many asynchronous stats operation as there are files in the list of files. The completion order of those operations may quite well be different from the original order of the list. This may substantially affect the error handling logic.

The following approach implements a state machine, which is aimed to executes stats asynchronously, yet sequentially (untested):

function openDir(path, callback) {
    path = __dirname + path;
    fs.exists(path, function (exists) {
        if (!exists)
            callback(null, null); // node (err, result) convention
        else {
            fs.readdir(path, function (err, files) {
                if (err)
                    callback(err, null); // node (err, result) convention
                else {
                    var results = [];
                    var i = 0;
                    nextStep(); // process the first file (the first step)

                    function nextStep() {
                        if (i >= files.length) // no more files?
                            callback(null, result); // node (err, result) convention
                        else {
                            fs.stat(path + files[i], function (err, stats) {
                                if (err)
                                    callback(err, null); // node (err, result) convention
                                else {
                                    results[i++] = stats;
                                    // proceed to the next file
                                    nextStep();
                                }
                            });
                        }
                    }
                }
            }
        }
    });                   
});

Promises may help to reduce the nesting level of the famous "Pyramid of Doom" like above.

edited Feb 14, 2014 at 6:21

answered Feb 14, 2014 at 4:43

noseratio

62.1k37 gold badges226 silver badges509 bronze badges

2 Comments

André Alçada Padez Over a year ago

promises, or the Async module :)

noseratio Over a year ago

@AndréAlçadaPadez, indeed, it's good to have a choice :) This presentation may help making it: trevorburnham.com/presentations/flow-control-with-promises

André Alçada Padez · Accepted Answer · 2014-02-13 13:18:57Z

1

try this:

function openDir(path, callback) {
    path = __dirname + path;
    fs.exists(path, function (exists) {
        var totalFiles = 0;;
        if (exists) {
            fs.readdir(path, function (err, files) {
                if (err) {
                    throw err;
                }
                var result = [];
                files.forEach(function (filename, index) {
                    fs.stat(path + filename, function (err, stats) {
                        if (err) {
                            throw err;
                        }
                        result[index] = filename;
                        totalFiles++;
                        if(totalFiles === files.length){
                            callback(result);
                        }
                    });
                });
            });
        }
    });
}

you can also use the Async module, to help on these kinds of situations

answered Feb 13, 2014 at 13:18

André Alçada Padez

11.7k26 gold badges72 silver badges130 bronze badges

2 Comments

plalx Over a year ago

I beat you 2 min ago =P

Ito Over a year ago

I use Async in many parts of my project. It is really useful and one of the best node packages ever seen.

Collectives™ on Stack Overflow

Make forEach asynchronous in JavaScript

3 Answers 3

Comments

2 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related