3

Here's my code:

function test(e, f) {
    for (var i = 0; i < e.length; i++) {
        $('#clickme').append("<button id='op" + i + "'>" + e[i] + "</button>")
        $('#op' + i).click(function () {
            f[i]();
        })
    }
}


$(function postPunk() {
    var func1 = function () {
        alert('1');
    }
    var func2 = function () {
        alert('2');
    }
    var func3 = function () {
        alert('3');
    }
    test(['Option1', 'Option2', 'Option3'], [func1, func2, func3]);
})

The click events aren't calling the functions. If I place an alert test in the click event, it fires fine.

Any ideas why this isn't working? It seems to be a problem with passing the array of functions as a parameter. Is there a better way to do this?

gen_Eric
  • 223,194
  • 41
  • 299
  • 337
Andrew Klatzke
  • 554
  • 2
  • 4
  • 15
  • 2
    didn't know `;` was optional in javascript. You are missing them at line 6, and the last line. – huysentruitw Aug 17 '12 at 18:12
  • Are you looking at it in a console? There's an error: `TypeError: [3] is not a function "f"+[i]();` – Jared Farrish Aug 17 '12 at 18:12
  • http://stackoverflow.com/questions/477700/array-functions-in-jquery Check the above question. This may help you. – Ramaraj Karuppusamy Aug 17 '12 at 18:15
  • There's more "missing" `;` than that (each of the function expressions). They are "optional", but it is best **not** to leave them off. – Jared Farrish Aug 17 '12 at 18:17
  • possible duplicate of [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) -- every day... – Felix Kling Aug 17 '12 at 18:20
  • 1
    @WouterH **Explicit semicolons are optional in most places in JavaScript** (for code that is already written cleanly with one-statement per line). This will not cause any problems with the above code. Look up "ASI" (Automatic Semicolon Insertion) for the rules. I write *almost no* semicolons in JavaScript; know the rules and be consistent. –  Aug 17 '12 at 18:21
  • @pst - Don't put words in my mouth. My opinion does not constitute "your code has errors". – Jared Farrish Aug 17 '12 at 18:25
  • @JaredFarrish I did not mean to imply you thought it was wrong (as in in syntactically invalid). Too many people just follow along blindly without justification for when to use (or not use) semi-colons in JavaScript. The "+2" on that first comment especially irritated me .. –  Aug 17 '12 at 18:27
  • @pst - That's exactly why I think it's a "best practice" for those to get in the habit of doing *until* they know the rules better. Syntactically, it's my opinion it's better to learn [strict mode](https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Functions_and_function_scope/Strict_mode?redirectlocale=en-US&redirectslug=JavaScript%2FStrict_mode) first, then decide on comfort level with different options. I agree, it is simplistic to blindly follow rules. But it's not wrong to defer to ending statements when you're learning. (Plus, it borks Tidy Up in jsFiddle...) – Jared Farrish Aug 17 '12 at 18:32

4 Answers4

8

As with every other question of this type, i keeps on changing.

Instead, try this:

for( var i=0; i<e.length; i++) {
    (function(i) {
        // your code that depends on i not changing
    })(i);
}
Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
4

This seems to be the classic JavaScript issue, where they all be the last value (or undefined since f[3] doesn't exist), since that's the value of i after the loop.

Try to pass the function reference directly to the click handler.

function test(e, f) {
    for (var i = 0; i < e.length; i++) {
        $('#clickme').append("<button id='op" + i + "'>" + e[i] + "</button>")
        $('#op' + i).click(f[i])
    }
}

Or, another solution is to make a function that returns a function. This will allow it to "close" around i.

function test(e, f) {
    var makeFunc = function(i) {
        return function() {
            f[i]();
        }
    };
    for (var i = 0; i < e.length; i++) {
        $('#clickme').append("<button id='op" + i + "'>" + e[i] + "</button>")
        $('#op' + i).click(makeFunc(i))
    }
}
gen_Eric
  • 223,194
  • 41
  • 299
  • 337
1

The code in the callback function uses the value of i after the loop has ended, so it points to an index outside the array. You need a closure in the loop so that each iteration gets its own instance of the variable:

function test(e, f) {
  for (var i = 0; i < e.length; i++) {
    $('#clickme').append("<button id='op" + i + "'>" + e[i] + "</button>");
    (function(i){
      $('#op' + i).click(function () {
        f[i]();
      });
    })(i);
  }
}
Guffa
  • 687,336
  • 108
  • 737
  • 1,005
0

Here's how I got it to work: http://jsfiddle.net/fH2Dk/3/

function test(e, f){    
     for(var i = 0; i < e.length; i++) {
         (function(i) {
            $('#clickme').append("<button id='op" + i + "'>" + e[i] + "</button>");
            $('#op' + i).click(function(){
                f[i]();
            });
        })(i);    
    }
}
Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
N. Taylor Mullen
  • 18,061
  • 6
  • 49
  • 72