jquery weird behavior

Question

Here's what i'm trying to accomplish: In the following mappings2 object, the keys are the name property of some grouped radio buttons and the value is a list which first string is a class that it's attached to a radio (might be more than one) of that group, let's call this radio button special-radio, and the second string is a class whose elements must be enabled when an special-radio is clicked, the code below is supposed to toggle between change events in a group of radios, letting execute some code when special-radio is clicked and a diferent code when other non-special radios from that group are clicked.

var mappings2 = {
        // Group's name            special-radio's class      other radios       
        'DependientesEconomicos': ['hijos-dependientes', 'cuantos-hijos'],
        'Preparatoria': ['otra-prepa', 'nombre-prepa'],
        'ReproboAlgunaMateria': ['reprobo', 'causas-reprobo']
        //'CausasReprobo': ['otra-causa-reprobo', 'otra-causa-reprobo-text']
    };

(function (maps) {
        for (grupo in maps) {
            $('input[name="' + grupo + '"]').change(function (e) {
                if ($(this).attr('class') === maps[grupo][0]) {
                    console.log(grupo + ' ' + $(this).attr('class') + ' true');
                    $('.' + maps[grupo][1]).attr('disabled', false);
                } else {
                    $('.' + maps[grupo][1]).attr('disabled', true);
                    console.log(grupo + ' ' + $(this).attr('class'));
                    $('.' + maps[grupo][1]).val('');
                }
            });
        }
    })(mappings2);

The code works fine for one entry for mappings2 but when you add more only the last entry works, and if you click one of the radios from previous groups the log shows the correct class, but grupo is not the group it belongs to, it shows the last entry's group, so that's the source of all the bugs, my question is why.

jquery 1.5.1

Thanks.

Answer 1

What's the problem?

The problem is caused by the way closures work. All of your 'change' functions reference the same grupo variable so whatever its final value is will be the value that all of the callbacks reference.

This is the same question/issue that has been described here (and has the same solution):
JavaScript closure inside loops – simple practical example

Detailed explanation:

Here is a simplified example showing the same issue:
http://jsfiddle.net/Sly_cardinal/PM2Gf/6/

HTML:

<div class="wrong" style="border: 1px solid red">
    <p>Incorrect: Clicking any item will show the last value of 'i'. i.e. class='11'</p>
    <div class="1">a</div>
    <div class="2">b</div>
    <div class="3">c</div>
    <div class="4">d</div>
    <div class="5">e</div>
    <!-- snip 5 other elements. -->
    <p class="result"></p>
</div>

Javascript:

// Setup click handlers for the 'incorrect' divs.
for (var i = 1; i <= 10; i++){
    $(".wrong ."+i).click(function(){
        var t = $(this).text();
        
        // This doesn't work because all of the handlers reference the same 'i' variable.
        // When the loop finishes 'i == 11' so when you click on any of the divs they output the current value of 'i' which is eleven.
        $(".wrong .result").text("You clicked div: class='"+i+"' content='"+t+"'");
    });
}

When you click on one of the divs we want it to print out its position in the list. So if you clicked on 'e' we want it to say that we clicked on the element with class='5'.

But this does not happen - every single item you click incorrectly says that it is class='11'.

The reason this happens is because all of the change functions reference the same i variable (just like your 'grupo' variable). So when the loop finally finishes i has the value 11. Therefore all of the divs print out the value of i which is 11.

---

How to fix it:

This shows how to change the way you create your event handlers to fix the error:
http://jsfiddle.net/Sly_cardinal/mr6ju/2/

Updated javascript:

// Setup click handlers for the 'correct' divs.
for (var i = 1; i <= 10; i++){
    
    // 1. Create an anonymous self-invoking function that
    // returns a function.
    var callback = (function(){
        
        // 2. Copy the current value of 'i' to a new variable so
        // that changes to 'i' cannot affect us.
        var currentI = i;
        
        // 3. Return the function definition that
        // will be bound to the event listener.
        return function(e){
            var t = $(this).text();
            
            // 4. NOTE that we reference 'currentI' and not 'i' here.
            $(".result").text("You clicked div: class='"+currentI+"' content='"+t+"'");
        };
    }());
    
    $("."+i).click(callback);
}​

To fix the issue we have to create an extra inner function that sets up a new closure with the current value of i. This inner function copies the value of i to a new variable called currentI and returns a new function that references currentI instead of i.

By copying the value to a new variable (which only exists inside the inner function) we prevent changes to i from affecting any other variable.

From these examples you should be able to adjust your code to create a new nested function that copies the value of grupo to a new variable and returns a new function that you assign as your event listener.