1

Forgive me for the broad title, I'm not sure how to state my problem.

I have a JS function that iterates through an array to create/append elements in a div. Each element is given an onclick function, however, when the onclick is executed, only the last iteration is recognized.

Here's an example of what I mean: (or check out this fiddle)

while(n--){
    myBtn = this.myButtons[n];
    btn = document.createElement("input");
    btn.setAttribute("type","button");
    btn.setAttribute("value",myBtn.name);
    btn.onclick = function(){
        window.alert(myBtn.msg);                  
    }
    div.appendChild(btn);
}

Here, I loop through an array of Javascript objects, create a button for each one and assign an onclick function which alerts the name of the object, however it always alerts the name of the last object instead of the name of the object which correlates with the given button. Check the fiddle to see what I mean.

How can I alter my fiddle to have it alert the correct message for each button?

I wrestled a bear once.
  • 22,983
  • 19
  • 69
  • 116
  • Need to make a closure around the myBtn value; search for "loop variables in anonymous functions" or something like that, it's a common mistake when starting out. – Dave Newton Aug 14 '14 at 22:20
  • @Dave: The event handler is already a closure. The "trick" is to create a new scope and capture the current value of the variable. Whether a closure is used or not does not matter. – Felix Kling Aug 14 '14 at 22:22
  • @FelixKling I read somewhere that I can create a new thread by using settimeout() and setting it to zero. Is that sort of what you mean? How would I go about creating a new scope? – I wrestled a bear once. Aug 14 '14 at 22:24
  • @FelixKling It's not a closure around the value of `myBtn` at the time of the loop iteration. – Dave Newton Aug 14 '14 at 22:26
  • @Adelphia: No, JavaScript is single threaded. `setTimeout` just lets you defer the execution of some code. Scope is created by calling a function. JS has only function scope. – Felix Kling Aug 14 '14 at 22:26
  • @DaveNewton: Yes it is. `myBtn` is not defined inside the event handler, hence it's a *free variable*. FWIW, theoretically every function in JS is a closure since every function has access to variables that are defined outside of it. Other languages might work differently and of course browsers optimize and don't store a reference to the environment if the function doesn't have free variables. – Felix Kling Aug 14 '14 at 22:27
  • 1
    You can also get this to work by assigning the onclick as a string attribute instead of a function reference `btn.setAttribute("onclick", "alert('" + myBtn.msg + "')");`. Updated fiddle: [http://jsfiddle.net/tvmhcw5f/1/](http://jsfiddle.net/tvmhcw5f/1/) – Dave Aug 14 '14 at 22:31
  • thanks for a creative solution @daveͤͮͣͩ that's probably not the approach I will take now that i've been introduced to the concept of closures, but if you would like to mark it as the answer I will accept it. – I wrestled a bear once. Aug 14 '14 at 22:36
  • @daveͤͮͣͩ: and there are many reasons why do avoid this. One of them is that you have to remember to escape the value, otherwise you might generate invalid JS. – Felix Kling Aug 14 '14 at 22:40
  • @FelixKling Yeah I can understand the string escaping could be an issue, but I am not sure I can think of "many" reasons to avoid this--especially in the example provided. I think in "many" cases this is a perfectly acceptable solution. – Dave Aug 14 '14 at 22:54
  • @Adelphia thanks for the comment upvote. Unfortunately I can't post an answer because the question is marked as a duplicate. No worries though – Dave Aug 14 '14 at 22:55
  • @daveͤͮͣͩ: http://stackoverflow.com/a/21975639/218196 (OK, it's just one reason ;) ). Another one would be that your solution would only work with primitive values. – Felix Kling Aug 14 '14 at 22:59

0 Answers0