Iterating parameters for a function within a loop

Question

JS Nooblord here, to give some context I have recently created my first JQuery based image slider to which I'm currently trying to generate a list of control buttons dynamically when the page loads.

I have succeeded thus far in creating the buttons but when it comes to writing the onclick function I'm having issues calling another function (with a parameter) inside a for loop.

I suck at explaining things but here is the code;

function addControls(){
    var x = document.getElementById('slider').childElementCount; 
    for (var i = 0; i < x; i++) {
        var ul = document.getElementById('slider-control');
        var li = document.createElement("li");
        var btn = document.createElement("Button");

        btn.onclick = function() { 
            goto(i); 
        };

        btn.appendChild(document.createTextNode(i + 1));
        ul.appendChild(li);
        li.appendChild(btn);
    }
}

function goto(index){
    alert(index);
}

Here is the JSFiddle preview.

What I expect is for each button to call the goto function with their respective position in the loop however every generated button with the onclick function uses the last index from the loop (4).

My initial thoughts are that the buttons are being rendered after the loops are finished and not within each iteration of the loop? also if anyone has any tips and alternatives for what I'm doing I would greatly appreciate that.

Thanks,

-Dodd

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

1

As commented on Mikelis Baltruks, you will have to use .bind.

You can use

goto.bind(null, i+1)

to map only index to it. If you wish to get the button as well, you can use

goto.bind(btn, i+1)

Sample JSFiddle

Bind

.bind is used to change the context of a function. Its syntax is

functionName.bind(context, argumentList);

This will create a reference of function with a newly binded context.

You can also use .apply for this task. Difference is, apply expects arguments as array and bind expect a comma separated list.

Note: both this function will just register events and not call it.

Reference

edited Jun 20 '20 at 09:12

Community

1
1

answered Sep 01 '16 at 10:32

Rajesh

24,354
5
48
79

Still wrapping my head about the syntax but .bind works a dream, thanks – Chris Dodd Sep 01 '16 at 11:06
@ChrisDodd I have tried to explain the syntax. Hope it helps. – Rajesh Sep 01 '16 at 11:16

Michelangelo · Answer 2 · 2016-09-01T10:38:14.980

0

You need to create a closure in the loop, this should work:

var x = document.getElementById('slider').childElementCount;
 for (var i = 0; i < x; i++) {
  (function (i) {
   var ul = document.getElementById('slider-control');
   var li = document.createElement("li");
   var btn = document.createElement("Button");
   btn.onclick = function() {
    goto(i);
   };
   btn.appendChild(document.createTextNode(i + 1));
   ul.appendChild(li);
   li.appendChild(btn);
  })(i);
}


function goto(index) {
  alert(index);
}

https://jsfiddle.net/g8qeq29e/6/

Or with ES6 let keyword;

function addControls(){
    var x = document.getElementById('slider').childElementCount;

    for (let i = 0; i < x; i++) {//change var to let here

        var ul = document.getElementById('slider-control');
        var li = document.createElement("li");
        var btn = document.createElement("Button");

        btn.onclick = function() { 
            goto(i); 
        };

        btn.appendChild(document.createTextNode(i + 1));
        ul.appendChild(li);
        li.appendChild(btn);
    }
}

function goto(index){
    alert(index);
}

edited Sep 01 '16 at 10:38

answered Sep 01 '16 at 10:29

Michelangelo

5,888
5
31
50

a little over the top when `btn.onclick = goto.bind(null, i);` fixes the issues just as easily - not sure why the other guy deleted his answer :p – Jaromanda X Sep 01 '16 at 10:31
@JaromandaX Why? `bind` does the same thing in the core. This shows better what is happening I think. – Michelangelo Sep 01 '16 at 10:33
@Mikey - your way is closer to the way I would do it, I was commenting because I was surprised by how simple it could be, and that the answer was then deleted!!! – Jaromanda X Sep 01 '16 at 10:34
@JaromandaX, there is even a better way in ES6. Will post also that answer. – Michelangelo Sep 01 '16 at 10:36
I seem to recall a bug in let in a for loop like that, however, as it's an ES2015+ answer, it's probably not a problem :p – Jaromanda X Sep 01 '16 at 10:42
@JaromandaX Strange it got deleted. It is fine now! Should be allright – Michelangelo Sep 01 '16 at 10:42
I saw it for a second!! it's still in the edit history :p – Jaromanda X Sep 01 '16 at 10:42

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

The problem is the reference to i.

for (var i = 0; i < x; i++) {
    var btn = document.createElement("Button");
    btn.onclick = function() { 
        goto(i);
        // any variable reference will use the latest value
        // so when `onclick` is actually run, the loop will have continued on to completion, with i == 4
    };
}

You need a separate variable to reference for each onclick handler. You can do this by creating a closure:

function makeOnclick(i) {
  // `i` is now a completely separate "variable",
  // so it will not be updated while the loop continues running
  return function() { goto(i); };
}

for (var i = 0; i < x; i++) {
    var btn = document.createElement("Button");
    btn.onclick = makeOnclick(i);
}

This can be done any number of ways, as others have shown. But this should explain why it's happening. Please ask any questions.

Iterating parameters for a function within a loop

3 Answers3

Bind

Reference