Dynamically generated Select-Elements and a JavaScript Closure

Question

I want to generate some <select elements dynamically. I'm copying an existing one and only change the name and the onchange-Event. This is where my problem is: Every <select> has its own ID. When the onChange-Event fires it should respond the Value and the ID of the Element. I don't know how to define the JavaScript Closure exactly. I tried it with "this.value", but apparently this does not work... I already found this example, but it did not work for me at all :/

The JavaScript:

var i = 0;
var selectArray = [];

function addSelector(){

var container = document.getElementById("check0");

selectArray[i] = document.getElementsByName("select0")[0].cloneNode(true);
selectArray[i].name = 'select'+i;
selectArray[i].onchange = function(v, i) {
        return function() {
           changeType(v, i)
        }
    }(this.value, i);

container.appendChild(selectArray[i]);
}

function changeType(selected, i) {
    switch (selected) {
        case 'One':
           alert(selected+' , '+i);
            break;
        case 'Two':
            alert(selected+' , '+i);
            break;          
        case 'Three':
            alert(selected+' , '+i);
            break;
        case 'Four':
            alert(selected+' , '+i);
            break;
        case 'Five':
            alert(selected+' , '+i);
            break;
    }
}

(I tried to post the HTML aswell, but for some reason i wasnt able to do this ;) )

Here is the fiddle

The value of `i` is never changing – Ruan Mendes May 29 '12 at 20:05 — Ruan Mendes, May 29 '12 at 20:05

score 1 · Accepted Answer · answered May 29 '12 at 20:12

this.value isn't known here:

function addSelector(){
    /* ... */
    selectArray[i].onchange = (function(v, i) {
        return function() {
            changeType(v, i)
        }
    })(this.value, i);
    /* ... */
}

this will be undefined (in strict mode) or window (normal mode). How should JavaScript know that this is a reference to the created/cloned <select> object? this will be correct inside of your onchange, so you don't have to include this by closure, only the index i has to be included:

selectArray[i].onchange = (function(index) {
    return function() {
        changeType(this.value, index)
    }
})(i);

Notice that your syntax for alert is wrong - you should use alert(selected + " " + i) instead, as the second argument will be ignored.

See also:

Thank you, thats what I needed to know! I wasnt sure how to say the changeType "take the value of the dynamically created selector". Thats why I thought i have to set "this.value" also as an parameter... Now it works just as i wanted. — cvoigt, May 30 '12 at 16:12

score 1 · Answer 2 · answered May 29 '12 at 20:15

Your example is really hard to follow. There are a number of problems with it

i is never changing
this.value does not have the correct value when you call it, but it's easy to figure out from within the handler

The following script does what I think you need http://jsfiddle.net/FNFQU/13/

function addHandler(node, index){  
   node.onchange = function() {
      alert('Changed select index ' + index + '. Its value is ' + node.value);
   }
}

function duplicateSelectElement() {
   var container = document.getElementById("check0");
   var allSelects = document.getElementsByName("select");
   var firstSelect = allSelects[0];
   var newSelect = firstSelect.cloneNode(true);
   addHandler(newSelect, allSelects.length + 1);
   container.appendChild(newSelect);
} 

addHandler( document.getElementById('select0'), 1 );

document.getElementById('btn').onclick = duplicateSelectElement;

Using the following HTML

<input type="button"
       id="btn"
       value="Add Selector!" />
<div id="check0">
<select id="select0" name="select">
    <option selected>One</option>
    <option>Two</option>
    <option>Three</option>
    <option>Four</option>
    <option>Five</option>
</select>
</div>

Thanks for your efforts. Indeed, i is never changing - I forgot to increment it. Your Script does what I wanted, but I need to do it using closures. Thats why I set the other Answer as correct. Thank you for your quick response! — cvoigt, May 30 '12 at 16:07
@vogti An answer that helps you find your problem deserves an upvote. My answer does use a closure, it doesn't use a self calling anonymous function. Why would you *need* a self calling anonymous function? — Ruan Mendes, May 30 '12 at 16:39
Your answer might use a closure, but the other answer helped me precisely to fix my problem. Thanks a lot again, i gave you an up! — cvoigt, May 30 '12 at 17:23
@vogti: Main reason why you shouldn't use the code you have: No need for the globals you created. Also, your `changeType` function is badly named (it doesn't change a type) but it also doesn't need a switch: `alert(selected + ',' + i)` would do it. — Ruan Mendes, May 30 '12 at 17:27

Dynamically generated Select-Elements and a JavaScript Closure

2 Answers2