Why using a loop in this case?

Question

I am a bit confused. Consider this case : I have a list of checkboxs, each one having an id, class and data-attribute (called data-uid, which is not equal to the checkbox id).

The point is to retrieve the clicked checkbox data-uid attribute.

So I use a JS click event on the checkboxs parent, to determine which one was clicked and get its data-uid.

People have told me to use a loop until I get the checkbox I clicked, like this (I could have used a normal for loop but you get the point) :

const checkboxArray = [...document.getElementsByTagName('input')];

document.addEventListener('click', event => {
    checkboxArray.find(element => {
        if (event.target === element) {
            console.log(element.getAttribute('data-uid'));
        }
    });
});

However, why not just do it this way, without any loop ?

document.addEventListener('click', event => {
    if (event.target.getAttribute('data-uid')) {
        console.log(event.target.getAttribute('data-uid'));
    }
});

Those two methods return the expected result : the data-uid attribute of the clicked checkbox. However, which one is the best to use and why ? As I said before, people always told me to use loops, but why if you can avoid it ?

Here is a snippet to see it in action :

const method1 = document.getElementById('method1');
const method2 = document.getElementById('method2');

const output1 = document.getElementById('output1');
const output2 = document.getElementById('output2');

method1.addEventListener('click', event => { 
  const checkboxArray = [...document.getElementsByClassName('checkbox')];
  
  checkboxArray.find(element => {
   if (event.target === element) {
    output1.innerHTML += `${element.getAttribute('data-uid')} `;
   }
  });
});

method2.addEventListener('click', event => {
 if (event.target.getAttribute('data-uid')) {
   output2.innerHTML += `${event.target.getAttribute('data-uid')} `;
  }
});

<div id="method1">
  <p><strong>Method 1 : Using a loop</strong></p>
  <label for="1">1</label>
  <input type="checkbox" data-uid="a1" class="checkbox">
  <label for="2">2</label>
  <input type="checkbox" id="2" data-uid="b2" class="checkbox">
  <label for="3">3</label>
  <input type="checkbox" id="3" data-uid="c3" class="checkbox">
</div>

<div id="output1">Data uid : </div>

<br>

<div id="method2">
  <p><strong>Method 2 : Not using a loop</strong></p>
   <label for="4">4</label>
  <input type="checkbox" id="4" data-uid="d4" class="checkbox">
  <label for="5">5</label>
  <input type="checkbox" id="5" data-uid="e5" class="checkbox">
  <label for="6">6</label>
  <input type="checkbox" id="6" data-uid="f6" class="checkbox">
</div>


<div id="output2">Data uid : </div>

Thank you very much for taking the time to read and enlighten me.

Tom

Note: `const checkboxArray = [...document.getElementsByTagName('input')];` uses non-standard features that are not present on all browsers; the `HTMLCollection` from `getElementsByTagName` is not specified as being iterable (unlike the `NodeList` from `querySelectorAll`, which is -- on modern browsers only). It works on some browsers, but not others. You can polyfill it, though, as I show in [this answer](https://stackoverflow.com/a/46057817/157247). — T.J. Crowder, Apr 26 '20 at 17:12
because its attaching click event to document so every click not just input will trigger and that code is possibly there to account only input. not saying its the best way but it is serving a purpose there — fastAsTortoise, Apr 26 '20 at 17:21
"*People have told me to use a loop*" - who told you this? Where? Did they give a reason? — Bergi, Apr 26 '20 at 17:21
@T.J.Crowder : Thank you for the suggestion. Will using something like Babel solve the problem all together ? — Tom687, Apr 26 '20 at 17:32
@fastAsTortoise : Excuse me but I do not fully understand what you are saying. The click event on document is only for showing the concept. What is serving a purpose here ? The loop ? Which way do you think is the best ? — Tom687, Apr 26 '20 at 17:32
@Bergi : Posts on the internet saying that confronted to an array you should loop through to get the desired data from. Also some of my ex co workers did told me that — Tom687, Apr 26 '20 at 17:33
@Tom687 I guess in that case they're right. "*When confronted to an array*" is the important part though. In your second approach, you don't have an array. — Bergi, Apr 26 '20 at 17:34
@Tom687 - I don't think Babel polyfills `HTMLCollection`, certainly not in a non-standard way. You could reliably use `const checkboxArray = Array.from(document.getElementsByTagName('input'));`. — T.J. Crowder, Apr 26 '20 at 17:37
@Bergi : You're right. But in a sense, in this context, you can choose to get an array or not. The point is that I was told that when in that kind of context, you should create an array (or nodeList) and loop through it. This is what confused me. — Tom687, Apr 26 '20 at 17:52
@T.J.Crowder : Thank you for your input on this question. I will use it this way as of now. — Tom687, Apr 26 '20 at 17:55
@Tom687 Then they're probably wrong, no you should try to avoid the loop if you can. But of course it's not clear what the actual context is, you said that you "*minimized the code example*". — Bergi, Apr 26 '20 at 17:56
@Bergi - I think I should have used the work 'simplified to the max' over 'minimized'. The script I actually work on and uses this kind of 'no loop thing' is composed of a lot of functions calling functions and is a few hundred lines long (it is a nodeJs chat app that I try to develop further as a kind of social network). Just wanted to know if the general principle is 'you should always loop' or 'no loop if you can avoid it'. Thank you for taking the time to help me and clarify things. — Tom687, Apr 26 '20 at 18:09

Bergi · Accepted Answer · 2020-04-26T17:35:40.463

2

It sounds like you only want to log things when a checkbox was clicked, not when any other element was clicked. If you do "have a list of checkboxes", then you need to test whether the clicked element is one of those checkboxes. The idiomatic way of doing that would be

document.addEventListener('click', event => {
    if (checkboxArray.includes(event.target)) {
        console.log(event.target.getAttribute('data-uid'));
    }
});

Sure, if you do not have a list/an array, but some other criteria (like "is a checkbox input" or "has a data-uid attribute"), then sure you can also use those to determine whether you want to handle the event. But the code does something different, and would fire on any element with that attribute not just on the inputs from your list. You would need to guarantee that only those have this attribute, to ensure that the two methods are equivalent.

edited Apr 26 '20 at 17:35

answered Apr 26 '20 at 17:25

Bergi

630,263
148
957
1,375

this is what i mean @Tom687 – fastAsTortoise Apr 26 '20 at 17:37
That is for sure. The scripts where I use this method (no loop) use a lot more criteria checking to make sure this is where the click is supposed to be done and targret the elements properly. I minimized the code example the most I could just to know if using a loop is necessary in that kind of context. – Tom687 Apr 26 '20 at 17:43
@Tom687 No, in your example case it's not necessary. – Bergi Apr 26 '20 at 17:46

score 1 · Answer 2 · answered Apr 26 '20 at 17:15

Your approach, without a loop, is definitely the smartest way to do it. Iterating over all the checkboxes to see if each one is the firing element is messy, and defeats the whole purpose of events and the Event.target property. The complexity also scales with the number of checkboxes on the page, where using Event.target requires only one operation no matter how many checkboxes there are.

Event.target.getAttribute('data-uid'), as you provided, is the most straightforward way of achieving the desired behavior. =)

Why using a loop in this case?

2 Answers2