March 10, 2013

Is it bad practice to shadow variables of a callback?

Question by hexacyanide

Take the asynchronous Node function fs.stat() for an example. If I need to use fs.stat() on a file, then do it again later, the result is shadowed.

fs.stat(file, function(err, stats) {
  fs.stat(file, function(err, stats) {
  });
});

The err variable, as well as stats variable is shadow – does this even matter if I won’t be using the first callback inside the second? Is it better practice to rename the second callback variables?

Does overwriting these variables, once, or multiple times have any performance impact?

Answer by Starx

Yes, it is bad practice. Depends on a developer viewing it of how bad it can get. If I got something like this, the first thing to pop in my mind would be

Why is he passing the err and stats to this callback?

fs.stat(file, function(err, stats) {
                        \    \
                         \    \
  fs.stat(file, function(err, stats) {
  });
});

When working in multiple instances of same function like you are doing right now. You have to make sure that all the variables can be accessed independently.

Unless you are sure one variable or construct is no longer is use, then you can re use that. But you have to mention this in a helpful comment to ensure another developer understands it.


Update:

Those variables on the callback functions have a scope limit, so they will not affect outside the function declaration.

An example of a valid case is:

  fs.stat(file, function(err, stats) {

  });

  //Another similar functions somewhere with same varialbes
  fs.stat(file2, function(err, stats) {
  });
...

Please fill the form - I will response as fast as I can!