Callbacks and variable scope

Question

I am working through the learnyounode tutorials and came across this interesting bit and I am trying to figure out how to handle scope when you use a variable as a callback function:

The following code works:

module.exports = function(filePath, fileExtention, callback){
  fs.readdir(filePath, function (err, list){
    if(err) return callback("Something went wrong", null) 
    var fileArray = []
    list.forEach(function(file){
      if(file.indexOf('.' + fileExtention) > -1) fileArray.push(file)
    })
    return callback(null, fileArray)
  })
}

While this code throws an exception of fileExtention not defined:

module.exports = function(filePath, fileExtention, callback){
  fs.readdir(filePath, cb)
}

var cb = function (err, list){
  if(err) return callback("Something went wrong", null) 
  var fileArray = []
  list.forEach(function(file){
    if(file.indexOf('.' + fileExtention) > -1) fileArray.push(file)
  })
  return callback(null, fileArray)
}

I am trying to understand why that variable is out of scope for the second call back function that is defined as cb and what I would do to fix this, as the signature of that callback is fixed.

Would the solution be make a local variable that is set to the fileExtention parameter?

This works, however I do not know if it is the proper way to handle the callback being passed into the module to maintain scope:

var fs = require('fs')
var fe = ""
var cb;
module.exports = function(filePath, fileExtention, callback){
  fe = fileExtention
  cb = callback;
  fs.readdir(filePath, findFiles)
}

var findFiles = function (err, list){
  if(err) return cb("Something went wrong", null) 
  var fileArray = []
  list.forEach(function(file){
    if(file.indexOf('.' + fe) > -1) fileArray.push(file)
  })
  return cb(null, fileArray)
}

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures — Paolo Moretti, Sep 18 '15 at 17:00

score 1 · Accepted Answer · edited May 23 '17 at 12:22

These two links will help you to understand better how scope and closure work in Javascript:

A good way to do what you want is by creating another annonymous function, and then sending the parameter for the function you've created. Something like that:

module.exports = function(filePath, fileExtention, callback){
  fs.readdir(filePath, function(err, list) {
    cb(err, list, fileExtension, callback);
  });
}

var cb = function (err, list, fileExtension, callback) {
  if(err) return callback("Something went wrong", null) 
  var fileArray = []
  list.forEach(function(file){
    if(file.indexOf('.' + fileExtention) > -1) fileArray.push(file)
  })
  return callback(null, fileArray)
}

score 1 · Answer 2 · answered Sep 18 '15 at 17:04

I am trying to understand why that variable is out of scope for the second call back function that is defined as cb and what I would do to fix this, as the signature of that callback is fixed.

It is out of scope because the function that is trying to use it is declared outside of the function which defined it.

Would the solution be make a local variable that is set to the fileExtention parameter?

A local variable … where?

Inside the exports function? That's what fileExtention already.

Inside the cb function? You still have to get the value from somewhere.

The solution is to do what you were doing in the first place, and put create your function inside the exports function.

Alternatively, you could make another function which contains both cb and the call to fs.readdir and then call that function from exports passing all the values you need from there as arguments.

This works, however I do not know if it is the proper way to handle the callback being passed into the module to maintain scope

As a rule of thumb: Avoid globals. That particular construct looks like it may be subject to race conditions.

score 0 · Answer 3 · answered Sep 18 '15 at 17:08

Short answer and how to fix it:

module.exports = function(filePath, fileExtention, callback){
  var fileExtention = fileExtention
  fs.readdir(filePath, cb)
}

var cb = function (err, list){
  if(err) return callback("Something went wrong", null) 
  var fileArray = []
  list.forEach(function(file){
    if(file.indexOf('.' + fileExtention) > -1) fileArray.push(file)
  })
  return callback(null, fileArray)
}

Is it best? Hell no!

Does it work? I think so!

Callbacks and variable scope

3 Answers3