0

I'm sorry if I'm missing something - I'm still learning Node's logic (esp. asynchronous jobs).

I want to download a zip file, then save it and unzip it. It looks like this:

request(URL)
    .pipe(fs.createWriteStream(/path/to/filename))
    .on('close', function () {
        // I want to unzip and I need access to filename
    });

Now the thing is, if I set a filename variable beforehand, since it's in a loop, by the time the file gets downloaded, filename will have the value of the last loop iteration.

Of course, I see we can do it by defining a function that would do all this, like fecthAndUnzip(URL,path). But my question is rather design-related: is it the only way we can do it in Node, or is there a better one? For example, can I access the filename in the on('close') event and if so, should I? Thanks!

Johy
  • 317
  • 1
  • 5
  • 16

2 Answers2

1

Don't know if it's helping you but the problem you describe in for loop has multiple solutions :

first one, use let instead of var to bind the variable to it's current scope :

for (let i = 0; i < 4; i++) {
    setTimeout(function() {
    console.log('test 2', i);
  }, 2000);
}

this will print 0, 1, 2, 3 even if loop will end before the first timeout callback is called.

You can use forEach instead of for loop if it's an array

[0, 1, 2, 3].forEach(function(i) {
  setTimeout(function() {
    console.log('test 3', i);
  }, 3000);
})

you can use an external function :

for (var i = 0; i < 4; i++) {
  setTimeout(function() {
    logI(i);
  }, 100);
}

function logI(i) {
  console.log(i);
}

the only thing that matter is that your variable needs to be bound to an internal scope to keep it's value even if the loop ends before your asynchonous function is called.

Gatsbill
  • 1,760
  • 1
  • 12
  • 25
  • Fantastic, it definitely answers my question. The only solution I had was #3 (which I ended up implementing), but I was looking for general practices for this use case. I'm glad to have learned the two others which I'll use a lot I think. Thanks! – Johy Dec 24 '17 at 16:30
1

Defining a separate function is definitely not the only way and there is a much better one.

You aren't showing the full code but I assume due to the behavior you are using var to define your variable. As in var filename = 'something'. When you do this you are defining the filename variable in the global [function] scope. This means it will be available to every part of the function regardless of where it was defined. In your case even though you define the variable in the loop it is scoped in the entire function, so every iteration you are actually overwriting that variable.

There is a good stackoverflow answer about scopes here if you want to learn more or if I didn't explain it well enough: https://stackoverflow.com/a/11444416/6359249

The solution would be to define a variable in the current block scope. If you are using a newer version of node.js that implement ES6 you can use let (or const for constant variables). Unlike var, this will define a function in the current block. If you define it in a loop it will only be accessible to that block and won't be changed. For example:

for (let i = 0; i < files.length; i++) {
  let filename = files[i];
  // Do stuff
}

Notice let i = 0. The same scoping concepts apply to loop variables. It's a common problem for people to use var i = 0 and become confused when using an asynchronous function and discovering i is the last value of i.

If you are not using ES6 you can define an anonymous function every loop:

for (var i = 0; i < files.length; i++) {
  var filename = files[i];
  (function (file) {
    // Do stuff
  })(filename);
}

Which is the same as the solution you described except you are creating and executing the function on the fly. If you aren't using ES6 for some reason then your original answer of creating a separate function is probably the best solution.

Froast
  • 742
  • 8
  • 16
  • Great, comprehensive answer too. My question was more design-related indeed (although I'm still curious about my specific use case with `fs.createWriteStream()`, i.e. if we can fetch the filename in `on('close')`). I am indeed using ES6, and that's exactly why I had this issue. I knew the function worked (that's how I used to do in the past), but I'm still learning ES6 and I believe it's important to rapidly integrate these concepts. Thanks, and thanks also for the link! – Johy Dec 24 '17 at 17:29
  • 1
    I see what you mean now, the answer there is no. The event is pretty much independent I believe and you are expected to have other information retrieved elsewhere. – Froast Dec 24 '17 at 18:30
  • Makes perfect sense and I learned best practices I didn't know on the way. Thanks much! – Johy Dec 24 '17 at 22:26