Attaching onchange function event to variable

Question

I recently got into closures and anonymous functions, and I'm wondering if my code is the right way to do it (it works!):

newInput.onchange = function(x){
     return function(){
          PassFileName(x);  
     }
}(counter);

so this is in a loop that "saves" the current 'counter' value (1,2,3...). If I didn't have the return function, then 'counter' will always be the last value of 'counter'.

Am I approaching this correctly with that code? or is there a better way to "capture" the current counter and attach it to an onchange event?

thank you!

Answer 1

What you're doing is fairly standard, and there's nothing wrong with it. However, there's a similar method which I prefer:

(function (x) {
    newInput.onchange = function () {
        PassFileName(x);
    };
})(counter);

To me, it is clearer by wrapping the whole block (and with the indentation) that I'm deliberating creating a new scope in order to capture the value of a variable.

Answer 2

Yes, you're approaching it correctly, but for maximum compatibility with implementations, you need to put parentheses around the function:

    newInput.onchange = (function(x){
//                      ^--- here
         return function(){
              PassFileName(x);  
         }
    })(counter);
//   ^--- and here

Whenever you're doing a function expression and calling it immediately, you need those parens because there's a parsing ambiguity otherwise.

---

Update:

While your approach is fine, it's worth pointing out that it's a bit profligate. ;-) It creates two functions on every iteration of your loop, the outer anonymous one, and the inner anonymous one. Both of those functions stick around (barring implementation optimisations, which you know Some Engines won't have). Instead, you could have just one per loop plus one factory function:

// In the loop
newInput.onchange = makeHandler(x);

// Outside the loop
function makeHandler(x){
     return function(){
          PassFileName(x);  
     };
}

Some may consider it easier to read (I certainly do), provided the makeHandler is still close enough to the loop that you don't lose track.

Using a factory function also offers you the opportunity of not closing over anything else in scope, although then you have to put the factory function further away (e.g., in a well-contained scope).

You might also consider a generic curry function, like the one provided by Prototype. A generic curry that doesn't pass on call-time arguments looks like this:

function curry(f) {
    var args = arguments;
    return function() {
        f.apply(undefined, args);
    };
}

But it's usually more helpful (but more expensive) to have one that does pass on the runtime arguments as well. Here's a cheap-and-dirty (not optimised; with optimisation call-time overhead can be markedly reduced):

function curry(f) {
    var args = Array.prototype.slice.call(arguments, 1);
    return function() {
        f.apply(undefined,
                args.concat(Array.prototype.slice.call(arguments)));
    };
}

The advantage in both cases is that you're not closing over anything new that you can avoid.

---

Off-topic: Also, technically, you're relying on the horror that is semicolon insertion (there should be a semicolon at the end of your return statement), which I always advocate not relying on. Pretty darned safe in this example, though. ;-)