0

I can't seem to figure this out. It's working fine for the first section of the for loop, but then the var is lost on the inner click function I commented where it breaks...

Plus, there's probably a cleaner way to write it to begin with:

$(function () {
var a = "rgb(58,88,90)";
var b = "rgb(123,156,158)";
for (var c = 1; c <= 3; c++) {
    if ($("#select" + c).is(":checked")) {
        $("#select" + c + "-icon").css("background", a);
        var d = document.getElementById("select" + c + "_title").innerHTML;
        $("#Selection_" + c).val(d)
    } else {
        $("#select" + c + "-icon").css("background", b);
        $("#Selection_" + c).val("Off")
    }
    $("#select" + c).click(function () {
    // here's where it stops working... var c is no longer recognized...
        if ($("#select" + c).is(":checked")) {
        $("#select" + c + "-icon").css("background", a);
        var d = document.getElementById("select" + c + "_title").innerHTML;
        $("#Selection_" + c).val(d)
    } else {
        $("#select" + c + "-icon").css("background", b);
        $("#Selection_" + c).val("Off")
    }
    })
}
return false });

Here are the first pair of objects it's targeting:

<label for="select1"><aside id="select1-icon" class="icon-form right rounded"><img src="../common/images/icon-viewDemo.png" /></aside>
                <input type="checkbox" id="select1" name="select" checked="checked" class="view" /> <h5 id="select1_title">Watch Demo</h5></label>

And:

<input type="hidden" id="Selection_1" name="Selection_1" value=""/>
matt _
  • 15
  • 4
  • How do you know that `c` is "no longer recognized"? What happens when you do an `alert(c)` there? – Marc Apr 14 '12 at 15:12
  • Hey, thanks for the quick response! I got a Null response with an alert just after the .click(function ()) line. – matt _ Apr 14 '12 at 15:15
  • Actually, just tried it again and it alerts with "4" – matt _ Apr 14 '12 at 15:19
  • How is that possible? Your loops should prevent it from going above 3. – Marc Apr 14 '12 at 15:20
  • You could accomplish this much more cleanly if you use classes instead of ids. Post your markup and I can show you how. – Brian Hadaway Apr 14 '12 at 15:28
  • I concur with @BrianHadaway. But the answer to your question is below from Mr. Murphy. You could also be much more efficient if you'd cache your selections vs. reselecting from the DOM every time. – Marc Apr 14 '12 at 15:35

2 Answers2

2

You are capturing your loop variable, so when the for loop is finished, the variable c has the value 4, which is the value the function sees when it executes.

var x;
for (var c = 0; c <= 3; c++) {
  x = function() { alert(c); };
}
x();

This will alert 4 because by the time you call x(), the variable c has the value 4.

If you want to capture the value of c rather than the variable itself, you can give each function a separate copy. I split the handler into a separate local function for readability.

function createClickHandler(c) {
    return function() {
        if ($("#select" + c).is(":checked")) {
            $("#select" + c + "-icon").css("background", a);
            var d = document.getElementById("select" + c + "_title").innerHTML;
            $("#Selection_" + c).val(d)
        } else {
            $("#select" + c + "-icon").css("background", b);
            $("#Selection_" + c).val("Off")
        }
    }
};
$("#select" + c).click(createClickHandler(c));

You can learn more about this phenomenon on this Web page and in this earlier stackoverflow question.

Community
  • 1
  • 1
Raymond Chen
  • 44,448
  • 11
  • 96
  • 135
0

c doesn't exist in the global scope so it doesn't exist when you click. If you made it global, it won't have the value you wanted when the click occurs. So use eval(); to replace the c with it's literal value, thus when clicked, you have the value you wanted.

$(
    function () {
        var a = "rgb(58,88,90)";
        var b = "rgb(123,156,158)";
        for (var c = 1; c <= 3; c++) {
            if ($("#select" + c).is(":checked")) {
                $("#select" + c + "-icon").css("background", a);
                var d = document.getElementById("select" + c + "_title").innerHTML;
                $("#Selection_" + c).val(d);
            } else {
                $("#select" + c + "-icon").css("background", b);
                $("#Selection_" + c).val("Off");
            }
            eval(
                '$("#select"' + c + ').click(function () {' +
                    'if ($("#select"' + c + ').is(":checked")) {' +
                        '$("#select"' + c + '"-icon").css("background", a);' +
                        'var d = document.getElementById("select"' + c +
                            '"_title").innerHTML;' +
                        '$("#Selection_"' + c + ').val(d)' +
                    '} else {' +
                        '$("#select"' + c + '"-icon").css("background", b);' +
                        '$("#Selection_"' + c + ').val("Off");' +
                    '}' +
                '});'
            );
        }
        return false;
    }
);
Robert Louis Murphy
  • 1,558
  • 1
  • 16
  • 29
  • `eval` is a dangerous function and should be avoided. It also messes up compiler optimizations. And the variable `c` does exist; it was captured by the closure. It just doesn't have the value you expect. – Raymond Chen Apr 14 '12 at 15:47