Using loop to bind click event over several elements not working in javascript/jquery

Question

I have a map of messages say:

var Mapping = {
"notnow": 2,
"expensive": 3,
"not_worth_it": 4
}

i have a bunch of html elements (lets say divs with the same name) so

<div id="notnow"></div>

,etc

now i want to attach a click handler to each of them, i run a loop as shown below

function setThemUp(){
   for(var item in Mapping)
   {
      $("#" + item).bind('click', function () {
      Apply(Mapping[item]); });
   }
}

But for some reason all of them seem to get bound to "not_worth_it":4. Not to their respective values.

I'm using Jquery 1.5.

Can someone please explain why this might be happening?

My guess is that instead of the Mapping[item] being resolved to their values, it's being passed as a reference or something, that's why since the value of item eventually points to "not worth it" all of them call the function with that value itself. Any way in which i could overcome them.

Hard coding each of them as

  $("#notnow").bind('click', function () {
      Apply(Mapping["notnow"]); });
  $("#expensive").bind('click', function () {
      Apply(Mapping["expensive"]); });
  $("#not_worth_it").bind('click', function () {
      Apply(Mapping["not_worth_it"]); });

does work, but i would prefer an elegant solution with a loop.

Answer

i went with the closure solution

function setThemUp(){
   for(var item in Mapping)
   {
       $("#" + item).bind('click', (function () {
            return function(temp) {
                 Apply(Mapping[temp]); };
            })(item));
       }
   }

Reasons being , this was more of a why the loop didn't work rather than about optimization of the jquery , since this was afterall a representative example, not my actual code, and this was an elegant solution to that problem.

You could just reference Mapping[this.id] in each of your click handlers. Also, it'd probably be better to give them all a class name so you can simplify the binding even further. — Shmiddty
– Shmiddty, Commented Nov 16, 2012 at 18:24

Elias Van Ootegem · Accepted Answer · 2012-11-16 18:29:12Z

4

This is a classic case of scope issues: you're referencing the variable item in each of your bound handlers. The variable item changes, though -> it's being assigned all properties of the Mapping object literal, the last one being not_worth_it.

Creating a closure might help, to preserve the state of item for each callback:

for(var item in Mapping)
{
   $("#" + item).bind('click', (function(currentItem)
   {//IIFE, pass item as argument---------/
        return function ()
        {//return function, \/ access to closure scope
            Apply(Mapping[currentItem]);
         };
    }(item)););
}

But this seems to be somewhat overkill, why not simply delegate the event, and use Mapping[$(this).attr('id')]?

edited Nov 16, 2012 at 18:29

answered Nov 16, 2012 at 18:13

Elias Van Ootegem

76.7k10 gold badges123 silver badges160 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Brijesh Bharadwaj Over a year ago

I can't use $(this).attr('id') because the actual ids are a lot more convoluted that what is shown here, they need some processing, infact you can forget the ids all together, im more interested in the fact that the state of item is being preserved and that Mapping[item] is processed at the time of the event instead of the parameters being resolved when i bind them itself.

Ian Over a year ago

@BrijeshBharadwaj Well this answer solves your problem. Notice how the function passed to bind is executed immediately, and inside it returns the actual function to be executed when the item is clicked. This way, the state of the item variable is kept accurate per event

Elias Van Ootegem Over a year ago

@BrijeshBharadwaj: you might want to think about using some data attribute to set the Mapping values, and delegate. After all, it's the most efficient way: one listener, one handler, all events. If not, a closure is what you need: the value of item is being passed to an IIFE, which returns the handler. The scope of the IIFE is not GC'ed and so the handler still has access to the value item HAD when the function was created

Shmiddty · Accepted Answer · 2012-11-16 18:26:52Z

0

I would suggest moving to this form:

Add the class mapped to your mapping divs.

HTML

<div id="notnow" class="mapped"></div>

JS

function setThemUp(){
    $('.mapped').bind('click', function () {
        Apply(Mapping[this.id]); 
    });
}

answered Nov 16, 2012 at 18:26

Shmiddty

13.9k1 gold badge38 silver badges52 bronze badges

Comments

RonaldBarzell · Accepted Answer · 2012-11-16 18:12:57Z

-1

The problem is you need to write:

for (var item in Mapping)

rather than foreach.

answered Nov 16, 2012 at 18:12

RonaldBarzell

3,8501 gold badge18 silver badges23 bronze badges

Collectives™ on Stack Overflow

Using loop to bind click event over several elements not working in javascript/jquery

3 Answers 3

3 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related