-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
  • 4,756
  • 2
  • 44
  • 70
user3679616
  • 19
  • 1
  • 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,246
  • 20
  • 88
  • 102
Amir Popovich
  • 28,017
  • 9
  • 48
  • 94
  • 1
    That is a pretty horrible solution *for jQuery*. Better to use a single handler against multiple matches (e.g. with a class selector). – Gone Coding 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 :) – Gone Coding 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
  • 35,563
  • 9
  • 45
  • 62
  • You unintentionally imply `data-` attributes are HTML5 only, which might put people off using them. They work just fine in HTML4 too :) – Gone Coding 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. – Gone Coding 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,126
  • 1
  • 11
  • 32