CODEWITHSUNDEEP
javascriptnode.js
Zach M.
Zach M.

Reputation: 1194

Callbacks and variable scope

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)
}

Upvotes: 0

Views: 52

Answers (3)

Adrian Lynch
Adrian Lynch

Reputation: 8494

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!

Upvotes: 0

Quentin
Quentin

Reputation: 943214

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.

Upvotes: 1

Buzinas
Buzinas

Reputation: 11725

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)
}

Upvotes: 1

Related Questions