0

I am running Asynchronous code with a foreach loop, but I need one step to be executed after the entire foreach is completed.

I've read many threads about this but I just can't seem to wrap my head around it to know what to do here.
For example, there is a lot of good information here: Using async/await with a forEach loop.
I am also not experienced with promises.

const fs = require('fs')
const path = require('path')

let sourceDir = 'srcDir/'
let destDir = 'D:/path/to/destDir/'

const isDirectory = source => fs.lstatSync(source).isDirectory()
const isFile = source => fs.lstatSync(source).isFile()
const getDirectories = source => 
        fs.readdirSync(source).map(name => path.join(source, name)).filter(isDirectory)
const getFiles = source => 
        fs.readdirSync(source).map(name => path.join(source, name)).filter(isFile)
const getData_File = file => file.substr(0,3)

getDirectories(sourceDir).forEach(dir => {
    let dirName = path.basename(dir)
    let newData = []

    getFiles(dir).forEach(file => {
        setTimeout(() => {
            newData.push(getData_File(dirName))
            console.log(file)
        }, 1000)
    })

    // If this would run after the above foreach loop, it will contain all data
    // However, it is in fact being run immediately and is therefore an empty array
    console.log(newData)

})

As described in the code above, I need the last line to run once for each directory after the entire foreach on files is completed. What is the best way to accomplish this?

R.J. Dunnill
  • 2,049
  • 3
  • 10
  • 21
DaveyD
  • 337
  • 1
  • 5
  • 15
  • Why do you need to make it asynchronous? if the above routines are all sync, there is no need to use async. That line of code specifically can be executed in another foreach loop, if that's needed. – briosheje Jun 18 '19 at 12:00
  • 1
    @briosheje `Why do you need to make it asynchronous?` That's how you get the best of of node, especially if this app is multi-user. – Keith Jun 18 '19 at 12:04
  • @Keith my question was **why** the above code should be async. From the question itself, it appears the OP is trying to make it async **just because he needs to execute that method**. It's not necessary to make it async for that task specifically, am I wrong? – briosheje Jun 18 '19 at 12:09
  • 1
    @briosheje `am I wrong?`, it depends were the OP is calling this. If he's using this say inside an Express app, then it's a big no no,.. The only time really in node you should be using any sync methods is say you was making a simple console application, but even then using async has it's benefits. – Keith Jun 18 '19 at 12:15
  • Were are you getting `getDirectories` and `getFiles` from, is this some NPM module your using?. – Keith Jun 18 '19 at 12:20
  • Thanks guys. I corrected and improved my post. I corrected 'synchronous' to 'Asynchronous'. I added the complete code (which I had left out for clarity purposes... :)), and I made it testable with console.log - And, I yes, I am using this in nodejs. Added that tag. – DaveyD Jun 18 '19 at 18:04
  • Now that you use the synchronous fs api for some reason, just drop the `setTimeout` and it will work? – Bergi Jun 18 '19 at 18:14
  • @Bergi, I used synchronous api because it is much cleaner looking to me rather than all the callbacks I would need. In any case, it's true that if I drop `setTimeout` it will work, but why - isn't `foreach` asynchronous? I.e., In my code I use nodejs rename which takes some time, if so, the console.log will occur before it the foreach is complete, correct? – DaveyD Jun 18 '19 at 19:52
  • @briosheje, I mentioned in my previous comment why I used Sync api - is that not a good reason? Otherwise, the amount of callbacks would be crazy - am I wrong? (Basically, my code is looping through files in folders, getting info on the files, writing the data to a file, and then renaming the file.) – DaveyD Jun 18 '19 at 19:54
  • 1
    @DaveyD No, `forEach` is not asynchronous, it doesn't wait for anything asynchronous happening in the callback, and even if it was asynchronous you wouldn't be waiting for the `forEach`. – Bergi Jun 18 '19 at 20:58
  • @Bergi, thanks for that. I did not realize that but now I do. I remember running into problems where the code at the end was triggered before the loop was complete, but it obviously was a different problem. - Another thing I _maybe_ learned, if it is true, that the problem in the forEach loop is when you run an async function inside it. This is why with setTimeout the problem occurred because setTimeout is async. Is this correct? If all this is true, I would be very happy that I finally understand how forEach works and Async functions! – DaveyD Jun 19 '19 at 19:25
  • 1
    @DaveyD yes, exactly. `forEach` is useless for asynchronous iteration. – Bergi Jun 19 '19 at 19:41
  • @Bergi, Thanks! Things are finally starting to make sense to me... – DaveyD Jun 21 '19 at 15:08

1 Answers1

0

If you need to execute asynchronous code within a loop like that you can consider wrapping the async code in a Promise. You can use Promise.all() to get the values returned by each Promise in the array and just set newData to that. Something like below: 

const files = ['file1', 'file2', 'file3', 'file4', 'file5']
let newData = [];

var promises = files.map(file => {
  return new Promise((resolve, reject) => {
    setTimeout(() => {
      resolve(file);
    }, 1000);
  });
});

Promise.all(promises).then(function(values) {
  newData = values;
  //Now that all the promises have resolved, we can set newData to our values arg
  console.log(newData);
});

//Expected newData to be empty here since it'll run before our async code
console.log(newData);
Tom O.
  • 5,730
  • 2
  • 21
  • 35