I'm having troubles understanding the logic when using for loops with eventlisteners in javascript

Question

I am learning web development and right now i'm working on vanilla javascript. I'm following a class on Udemy and everything is going well but there is a challenge where we are supposed to build a drumkit so everytime we click on a button it should trigger a function. So the solution the instructor is giving is using a for loop

var nbButtons = document.querySelectorAll(".drum").length;

for(i = 0; i<nbButtons; i++){
    document.querySelectorAll(".drum")[i].addEventListener("click", handleClick);
            
    function handleClick() {
        alert("I got clicked!" + i);
    }
}

So if I try to analyze the code I understand than :

We create a variable to keep track of the number of .drum elements (querySelectorAll(".drum") returns an array of all .drum elements and we get it's length. We start a loop : We start listening for clicks on the .drum element whose number is equal to i and start the function handleClick.

So I tried it and it works but I don't understand why. the loop starts when the page is loaded which means i = nbButtons very quickly (i added console.log in the loop and it does) so logically it should listen only on clicks on the last element ?

I know i'm missing something but I don't know what.

Thanks in advance

score 0 · Answer 1 · answered Jul 18 '20 at 10:17

nbButtons is just the number of buttons, say 5 buttons.

Then you use a for loop to go through all the 5 buttons. The loop executes 5 times, and this number is temporarily stored in the i variable. So, using i you can also get to each button and add the event listener to that button.

You can log this with

for(i = 0; i<nbButtons; i++){
    console.log(i);
    console.log(document.querySelectorAll(".drum")[i])
}

NullDev · Accepted Answer · 2020-07-18T10:23:33.207

0

We start listening for clicks on the .drum element whose number is equal to i and start the function handleClick.

This is the part you misunderstood. You don't immediately start listening to the current element. This line registers the listener which then listens more or less in the background. So in your for loop you register an event listener for each array index. The event listener isn't overwritten on the next iteration. You're able to register multiple event listeners at once, even on the same element.

You can verify that in devtools:

More information on what addEventListener() does: https://www.w3schools.com/js/js_htmldom_eventlistener.asp

edited Jul 18 '20 at 10:23

answered Jul 18 '20 at 10:17

NullDev

6,739
4
30
54

Oh ok it makes total sense now, I did assume each one was replacing the previous one. Thank you so much for your answer. – altho Jul 18 '20 at 10:34
@altho No problem! Glad I could help :) – NullDev Jul 18 '20 at 10:35

score 0 · Answer 3 · answered Jul 18 '20 at 10:44

There are several parts of your code that I would write differently (some pointed out in other answers), but the key problem is that you have a global variable i, which will already have the value nbButtons by the time any of the handleClick functions actually run - so it's no surprise that this is the value you see logged on each click.

To make sure the correct value of i is logged for each button, you have to ensure the function you add as the event handler actually uses the "current" value. Here is one way:

var nbButtons = document.querySelectorAll(".drum").length;

for(let i = 0; i<nbButtons; i++){
    document.querySelectorAll(".drum")[i].addEventListener("click", function() { handleClick(i);} );
} 

function handleClick(i) {
    alert("I got clicked!" + i);
}

There are a few key differences from your faulty code. For one, I've pulled the definition of the handleClick function out of the loop - this doesn't actually matter here (function declarations are scoped to functions, not arbitrary blocks like loops), but is neater and matches better how the code actually behaves. More importantly, I've made it take a parameter, i, and made sure the correct i is passed in in each addEventListener call.

And most importantly of all, I've replaced your global variable i in the loop with a local one, declared with let. This ensures that when the button is clicked and the function - function() { handleClick(i);} is called, it sees not a global i whose value was long ago incremented past the intended value, but one scoped to the particular loop iteration, which therefore only ever had the one value. Note that let is crucial to make this work, using var would not (without some extra complications anyway) - see this classic question for more detail

Thank you, i'm looking into your link. – altho Jul 18 '20 at 18:04 — altho, Jul 18 '20 at 18:04

score -1 · Answer 4 · answered Jul 18 '20 at 10:27

-1

What you have done in your for loop is search for a ".drum" using the queryselectorall function which I do not think is efficient - if you must loop with "for";

I think you should: var nbButtons = document.querySelectorAll(".drum"); var nbButtonsLen= nbButtons.length;

for(i = 0; i<nbButtonsLen; i++){
    nbButtons[i].addEventListener("click", handleClick);
}
function handleClick() {
    alert("I got clicked!" + i);
}

answered Jul 18 '20 at 10:27

Kjut

56
4

While I agree it's better rewritten this way, this does nothing to address the actual problem. – Robin Zigmond Jul 18 '20 at 10:45
Thank you. Can you explain why it is better to do this way ? – altho Jul 18 '20 at 18:03
Basically so your script doesn't have to keep computing `document.querySelectorAll(".drum")` every time through the loop. And you don't have to type it more than once! – Robin Zigmond Jul 18 '20 at 18:29

I'm having troubles understanding the logic when using for loops with eventlisteners in javascript

4 Answers4