Creating multiple functions with for loop

Question

First, I did find a couple links that appeared to address this problem, but my understanding of javascript (and code in general) is pretty bad, and the solutions/explanations were difficult for me to generalize here. I know it has to do with a closure (which I vaguely understand).

Background: I created 10 checkboxes using a loop, each with 4 radio buttons that are supposed to appear when the checkbox is checked. I can accomplish the "appear/disappear" by creating 10+ functions one-by-one, but I would prefer to create my ten functions with a loop. The code below shows how each checkbox is calling a variable function on change (e.g., t0Disp()).

<?
for ($row = 0; $row <10; $row++)
{
  $topic = $topic[$row];
  ?>
  <input id="C<? echo $row ?>" type="checkbox" value="true" onchange="t<? echo $row ?>Disp()" /><? echo $topic ?>
  <br />
  <div id="<? echo $row ?>div" style="display: none">
      <? for ($col = 0; $col < 4; $col++) 
    {
      ?>
      <input type="radio" name="r<? echo $row ?>" value="<? echo $col ?>" onchange="disable<? echo $col ?>(); disable<? echo $row ?>();" />
      <? echo $col+1; ?>
    <?
    }
    ?>
      <br />
      <br />
  </div>
<?
}
?>

The code above works fine if I manually input my t0Disp() through t9Disp() functions. When I tried making a for-loop to create these functions (below), the function only works for t9Disp()

for (var i=0; i<10; i++) {
    var idiv = i + "div";
    var eldiv = document.getElementById(idiv);
    var ci = "C" + i;
    var elci = document.getElementById(ci);
    window['t' + i + 'Disp'] = function() {
        if(elci.checked == true) {
            eldiv.style.display = "inherit";
        }
        else {
            eldiv.style.display = "none";
        }
    }
}

I just need to understand why the javascript loop doesn't just spit out 10 distinct functions in the way that my php loop spits out 10 distinct checkboxes?

Thank you!

Possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — VLAZ, Sep 29 '16 at 16:22
In addition to the duplicate, I don't even know why you want 10 functions, instead of simply 1 that takes a parameter. — VLAZ, Sep 29 '16 at 16:23

Sean Vieira · Answer 1 · 2016-09-29T16:30:37.853

Your problem is that var is hoisted, so you do get ten distinct functions, each one pointing at one shared variable ... which points at the last set of checkboxes.

The first solution to your problem is to use let or const (if you only need to support modern browsers):

for (var i=0; i<10; i++) {
    const idiv = i + "div";
    const eldiv = document.getElementById(idiv);
    const ci = "C" + i;
    const elci = document.getElementById(ci);
    window['t' + i + 'Disp'] = function() {
        if(elci.checked == true) {
            eldiv.style.display = "inherit";
        }
        else {
            eldiv.style.display = "none";
        }
    }
}

However, there is even more you can do:

const checkboxes = document.querySelectorAll('.js-showChoices');
[].slice.apply(checkboxes).forEach(checkbox => {
  checkbox.addEventListener('change', event => {
    if (event.target.checked) getDivFor(event.target).style.display = 'inherit';
    else getDivFor(event.target).style.display = 'none';
  });
});

function getDivFor(checkbox) {
  // Find div based on checkbox and return it
}

Thank you, this is great! The second box of code is way beyond me, but it certainly looks more elegant than what I've been doing. I think I'm going to stick with Rayon's solution, though. — danny, Sep 29 '16 at 17:59

score 0 · Accepted Answer · answered Sep 29 '16 at 16:24

0

You do not need multiple function, just one function with argument can do it for you!

function disp(elci, eldiv) {
  eldiv = document.getElementById('eldiv');
  if (elci.checked == true) {
    eldiv.style.display = "inherit";
  } else {
    eldiv.style.display = "none";
  }
}

And your markup will be:

<input id="C<? echo $row ?>" type="checkbox" value="true" onchange="Disp(this,'<? echo $row ?>div')" /><? echo $topic ?>

answered Sep 29 '16 at 16:24

Rayon

36,219
4
49
76

This is so smart! I don't know why I wasn't thinking this way, as it is way more in line with what I'm capable of coding... Thank you! – danny Sep 29 '16 at 17:58

score 0 · Answer 3 · answered Sep 29 '16 at 16:36

The problem with your code is that when functions get added those remember last values of variable eldi, elci.

You can make use of closure to remember appropriate values of variables. For that do something like this

function addFunction(name, elementDiv, elementCi) {
  window[name] = function() {
    if(elementCi.checked == true) {
        elementDiv.style.display = "inherit";
    }
    else {
        elementDiv.style.display = "none";
    }
 }

}

for (var i=0; i<10; i++) {
   var idiv = i + "div";
   var eldiv = document.getElementById(idiv);
   var ci = "C" + i;
   var elci = document.getElementById(ci);
   addFunction('t' + i + 'Disp', eldiv, elci);
}

score 0 · Answer 4 · answered Sep 29 '16 at 16:47

Sean has the problem right though there would be an easier way to do it than consts and would support previous browsers

for (var i=0; i<10; i++) {
    window['t' + i + 'Disp'] = (function() {
        var idiv = i + "div";
        var eldiv = document.getElementById(idiv);
        var ci = "C" + i;
        var elci = document.getElementById(ci);

        return function() {
            if(elci.checked == true) {
                eldiv.style.display = "inherit";
            } else {
                eldiv.style.display = "none";
            }
        };
    })();
}

JsFiddle

This works by storing the variables in scope inside the first function that's only accessible by the returned function.

Rayon has a better solution than the looped functions though, a function that can handle any row given to it means it'll expand or contract to your needs.

Creating multiple functions with for loop

4 Answers4