Async operation inside callback of event

Question

I'm working with a module called findit, which recursive find files within a target folder and return the events "file", "error", "end":

const finder = findit("folder/");
finder.on("file", (file) => {});
finder.on("error", ((error) => {});
finder.on("end", () => {});

The end event will be called when the finder finds all files... but, inside the file event i'm making an async operation, that separate files only if they have something inside:

const finder = findit("folder/");
let neededFiles = [];

finder.on("file", (file) => {
  // async operation here to store only the files that i want
  // neededFiles = [...neededFiles, file];
});

finder.on("error", ((error) => {});

finder.on("end", () => {
  console.log(neededFiles); // empty array
});

The neededFile will be empty because the async operation has not finished yet. My question is: What chances do i need to do to wait the async operation in the end event?

Thanks.

Are you familiar with promises? If so, I would recommend that you accumulate an array of promises in your `finder.on` callback and then use Promise.all(arrayOfFileLoadingPromises).then(...) inside of the end event. — barry-johnson, Mar 25 '17 at 18:46
I am. This is a perfect idea! Do you want answer it to win the points? — , Mar 25 '17 at 18:50
I was going to just say use Thomas' answer, but I am not sure why he has wrapped the whole mess in the Promise constructor antipattern. — barry-johnson, Mar 25 '17 at 19:03
[Quick explanation here](http://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) from a guy who knows Promises very, very well. Worth noting - it's fine in that it does work and is valid, etc (and at times is unavoidable), but it is increasingly avoided. — barry-johnson, Mar 25 '17 at 19:14
It's actually the only way to make it work cleanly in this case, so while it is increasingly not seen in user (vs library) code, depending on how you want to actually use the code above, Thomas' example is perfectly reasonable. His assumption was that you were calling this from other code and might want the final result as a promise. My approach was just to use promises internally, as if you were running the above file as single-file script/utility. — barry-johnson, Mar 25 '17 at 19:23
@barry-johnson In this case I wouldn't call out on the antipattern. Sure, one could do `new Promise(… finder.on("end", () => resolve(files)); … }).then(Promise.all)` but that's not a that great advantage — Bergi, Mar 25 '17 at 19:26
I agree, which is why I added my additional comment. As I further said, the issue is whether the user wants a promise at the end or is just running as a one shot. — barry-johnson, Mar 25 '17 at 19:29

score 1 · Accepted Answer · answered Mar 25 '17 at 18:58

Since you've provided only a scratch of you app, I tried to build around that, and show how you could wrap that into a promise to handle the async part.

function find(folder){
    return new Promise((resolve, reject) => {
        const finder = findit(folder);
        const files = [];

        finder.on("file", (file) => {
            //do stuff like 
            //files.push(valueOrPromise);
            //or
            //files.push( find(anotherPath) );
        });
        finder.on("error", reject);

        finder.on("end", () => {
            //this line finally "returns" the result.
            //to this point you can modify files as you wish
            resolve( Promise.all(files) );

            //hint, if you're working with recursive stuff, you may have nested Arrays, so you should flatten the result
            //resolve( Promise.all(files).then(values => values.reduce((a,b) => a.concat(b), [])) )
        }); 
    })
}

Usually people ask at this point: why do I need promises? Because they implement state management of async tasks; so why would you want to implement it yourself?

score 0 · Answer 2 · answered Mar 25 '17 at 18:38

0

And why no use the 'end' inside the 'file'? some like:

finder.on("file", (file) => {
 // async operation here to store only the files that i want
 // neededFiles = [...neededFiles, file]; // GENERATE A PROMISE SO U CAN CHAIN THEN FUNCTION
 neededFilesPromise.then(function(neededFiles){
   finder.on("end", () => {
      console.log(neededFiles); // empty array
   });
 }).catch(function() {
   finder.on("error", ((error) => {});;
 })

});

answered Mar 25 '17 at 18:38

Giancarlos

151
10

1

One reason might that it will add an end event listener every time it finds a file – barry-johnson Mar 25 '17 at 18:45

score 0 · Answer 3 · answered Mar 25 '17 at 19:11

I would say this is the perfect use case for Promise.all(). So something like this. You might want to 'promisify' your file operations so it is even cleaner (if you are using something like bluebird to replace the native promises). I am not sure if your lib is return the content of the file or a filename

const fs = require('fs');
require('bluebird').promisifyAll(fs);
const finder = findit("folder/");
let neededFiles = [];

finder.on("file", (file) => {

    // if file is the file content
    neededFiles.push(/* result of your op above */);
    // if file is a file name
    neededFiles.push(fs.readFile(/* file name */));
});

finder.on("error", ((error) => {});

finder.on("end", () => {
  Promise.all(neededFiles)
     .then((nf) => {
            console.log(nf); // you should now have something here...
           });
});

Async operation inside callback of event

3 Answers3