2

I have separate elements on my DOM with ID of "inner_1" up to "inner_20" e.g. inner_1, inner_2, inner_3, ect..

I made a javascript loop that will create separate event listeners functions to target each element, but it's assigning the wrong value to each function.

My loop in my .js file

for (section = 1; section < 21; section++) {
  $('#inner_'+section).click(function(event) {
    alert('Hooray!'+section);
    event.preventDefault();
  });
};

It outputs

=> "Hooray!21"

For every element I click on, regardless if it's "inner_1" or "inner_15".

How can I make it display the correct number for every function made e.g. "inner_14" will have alert of "14"

Thank you!

  • In addition to the answers, consider a real simple solution which is `for (let section=` More basically, however, using constructed IDs as a way to address DOM elements is an anti-pattern. You'll spend the result of your life doing string arithmetic to create IDs and then tracking them down in the DOM and then in some cases picking the IDs back apart again. It's much better, for example, to identify the `inner` elements with a class, and then grab them all with `getElementsByClassName` or `querySelectorAll`, then loop through the results to add the event listener. –  Oct 27 '15 at 03:13
  • Wow, this technique will prove invaluable to me! This kind of thinking will contribute to my becoming a better developer!! Thank you for your tips and guidance torazaburo! –  Oct 27 '15 at 03:24

3 Answers3

1

The output will always be "Hooray!21" because that is the value of section when the click handler gets invoked. The for loop will have already finished iterating causing section to be "fixed" in 21. Instead try this

for (section = 1; section < 21; section++) {
  (function(s) { 
      $('#inner_'+s).click(function(event) {
        alert('Hooray!'+s);
        event.preventDefault();
      });
  })(section);
};
Griffith
  • 3,229
  • 1
  • 17
  • 30
  • Create a new scope is not necessary, just pass the data to the event: `$("#inner_" + s).on("click", { index: s }, function(e) { alert(e.data.index); })` – jherax Oct 27 '15 at 02:23
1

Loop not appear necessary. Try using attribute starts with selector

$("[id^=inner]").click(function(e) {
  e.preventDefault()
  alert("Hooray!" + this.id.split("_")[1])
})
guest271314
  • 1
  • 15
  • 104
  • 177
  • Very clean. This would be preferred over programmatically generating 21 anonymous functions. – Josh Beam Oct 27 '15 at 01:44
  • WOW this is wizardry! I chose the answer by Griffith as it defined why my loop didn't work, but I'll be using this code! Awesome! Thank you! –  Oct 27 '15 at 01:48
  • Yeah this is definitely an improved code. – Griffith Oct 27 '15 at 02:25
0

Think in refactor the code, if you are using jQuery, then delegate just one event handler for all elements, thus, you avoid create a lot of event handlers that will do the same thing.

Consider the following structure:

<section class="parent-section">
    <div id="inner_1">1</div>
    <div id="inner_2">2</div>
    <div id="inner_3">3</div>
</section>

Now we are going to delegate the event handler to the parent element, that way only one event handler will be created:

$(".parent-section").on("click", "[id^='inner_']", function(e) {
    e.preventDefault();
    var item = this.id.match(/\d+$/);
    if (!item) { return; }
    alert(item[0]);
});

This way you will get a improvement in memory optimization, because only one event handler will handle the click event in all children elements.

jherax
  • 5,238
  • 5
  • 38
  • 50