-1

I would like to ask something about set event trigger with jquery.

First of all, i have a form with many buttons ( btnadd1 , btnadd2, btnadd3 , ... )

I have this code:

for(i=1;i=<3;i++){

 $('.btnAdd'+i).click(function(){
    alert(i);
 });
}

The code should be alert(1) when i click btnadd1 , alert(2) when i click btnadd2, but the problem is when i click btnadd1, btnadd2, or btnadd3 it alerts "3"

My assumption is that the function overwrites the previous function. Does anyone have a solution for this problem?

mirabilos
  • 5,123
  • 2
  • 46
  • 72
  • possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – JJJ Jan 13 '15 at 10:46
  • 4
    This is a terrible pattern to use. Use a common class and assign a single click handler. You are completely negating the entire point of having classes. – Rory McCrossan Jan 13 '15 at 10:47
  • see http://robertnyman.com/2008/10/09/explaining-javascript-scope-and-closures/ and scroll to the section : The infamous loop problem – errand Jan 13 '15 at 10:52

3 Answers3

1

This is a common closure issue. You can work around it like this:

$('.btnAdd'+i).click(function(idx){
   return function(){alert(idx);};
 }(i));

JSFIDDLE

JJJ
  • 32,902
  • 20
  • 89
  • 102
Amir Popovich
  • 29,350
  • 9
  • 53
  • 99
  • 1
    That is a pretty horrible solution *for jQuery*. Better to use a single handler against multiple matches (e.g. with a class selector). – iCollect.it Ltd Jan 13 '15 at 10:53
  • @TrueBlueAussie - What your saying is correct - He wanted to know the problem with his code and I provided him it. There are lot's of better ways to solve "problems" like this. ECMA6 will solve it with a new operater let. – Amir Popovich Jan 13 '15 at 10:56
  • 1
    It's good to explain what is wrong, but then provide a good solution showing how it should be done instead :) – iCollect.it Ltd Jan 13 '15 at 10:58
0

Avoid using closure inside a loop...better is to assign a single class to all the buttons and register single event for that..make use of data attributes.

$('.someclass').click(function() {
  alert($(this).data('value')); //using jquery's data function
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<button class="someclass" data-value="1">
  button1
</button>
<button class="someclass" data-value="2">
  button2
</button>

there are other solution to achieve what you want, using attribute selector [class^="btnadd"] and so on but i think this solution is better and readable.

bipen
  • 36,319
  • 9
  • 49
  • 62
  • You unintentionally imply `data-` attributes are HTML5 only, which might put people off using them. They work just fine in HTML4 too :) – iCollect.it Ltd Jan 13 '15 at 10:59
  • :)...as far as i know `data` attribute was introduce in HTML5 only.. but anyways.. i agree with you so deleting HTMTL5 from there.. thanks @TrueBlueAussie – bipen Jan 13 '15 at 11:00
  • `Hoja.M.A` is correct. You have a couple of typos in your example. Best fix and provide a JSFiddle so I can upvote. – iCollect.it Ltd Jan 13 '15 at 12:04
  • oopss...didn't see it.. need a coffee now i guess... thanks guys anyways i think there is no more typos now..added snippet too... :) – bipen Jan 13 '15 at 12:11
0

The issue deals with closure inside loop.

Better you could use a user defined attribute in your button and use a single classs for binding the event. see @bipen's answer.

<button class= "someclass" data-value="1"> click</button>
<button class= "someclass" data-value="2">click </button>



 $('.someclass').click(function(){
     alert($(this).attr('data-value'));
  });

Check Fiddle

http://jsfiddle.net/hoja/3jt5furd/5/

Hoja
  • 1,057
  • 1
  • 12
  • 32