JavaScript scope - getting right instance of an object for onClick in loop

Question

Sorry for the poor title, hard to explain this without showing the code.

I have a javaScript class called Coupon that has a function that basically opens up a fancyBox and displays the contents of that coupon object (its dynamic).

Like so

function CouponPopup( id, title, barcodeUrl, expirationDate, description ) {
  this.coupondId = id;
  this.couponContainer = document.createElement('div');
  this.couponTitle = document.createElement('h2');
  this.couponPrizeName = document.createElement('h3');  
  var self = this;
  // More vars, etc. . . 

  // assemble the coupon
 this.couponContainer.appendChild(this.couponTitle);
 this.couponContainer.appendChild(this.couponPrizeName);
 this.couponContainer.appendChild(this.couponRevealDetails);
 this.couponContainer.appendChild(this.couponBarcode);
 this.couponContainer.appendChild(this.couponExpirationText);
 this.couponContainer.appendChild(this.couponPrintButton);

 this.show = function ( event ) {
     $.fancybox(self.couponContainer);
  }
};  // end class CouponPopup

Now, in a the code calling this, I am trying to make a link - that when it is clicked will call the show function on the right instance of CouponPopup.

for (var i = 0; i < dataLength; i++) {
   if (data.type === "coupon") {
       var couponId = "coupon_" + i;
       // right now, my coupon will be on global scope.
       myCoupon =  new CouponPopup( couponId,
           data.rewards[i].prize_name,
           data.rewards[i].url,
           data.rewards[i].expirationdate);

       rewardInfo = '<a id="' + couponId + '" onclick="myCoupon.show( event )">View Coupon</a>'; 

   }
}

rewardInfo eventually gets appended to a the page - and looks fine.

However, when I click on it, since myCoupon is on the globalScope - the fancyBox that is shown is always the last coupon that is created.

If I try to put myCoupon on the local scope (adding the var keyword), I get an JS error saying that myCoupon is undefined when its added to the onClick handler of the link.

What I want to do is make myCoupon a local variable and still have this work - so the onClick event of the link is tied to the specific instance of myCoupon

What is the best way to do this? I have a hack working, but its a hack and I really don't like needing to use global variables in my code if I can help it.

Having a brain fart on this one, so any help is appreciated!

Looking for an old-school ES5 solution.

Possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — Heretic Monkey, May 01 '17 at 20:56
@MikeMcCaughan Not exactly duplicate. Here actually the object's reference remains same. Closure is not playing a role I think. — binariedMe, May 01 '17 at 21:03

binariedMe · Answer 1 · 2017-05-01T20:57:57.790

You may try doing the following :

for (var i = 0; i < dataLength; i++) {


    if (data.type === "coupon") {
       var couponId = "coupon_" + i;
       // right now, my coupon will be on global scope.
       myCoupon =  new CouponPopup( couponId,
           data.rewards[i].prize_name,
           data.rewards[i].url,
           data.rewards[i].expirationdate);

       rewardInfo = '<a id="' + couponId + '" onclick="showCoupon('+ couponId +')">View Coupon</a>'; 

     }
}

And then create a function showCoupon like this :

  function showCoupon (couponId) {
    try {
      var index = parseInt(couponId.replace('coupon_', ''));

      new CouponPopup( couponId,
           data.rewards[index].prize_name,
           data.rewards[index].url,
           data.rewards[index].expirationdate).show();
    } catch(err) { console.log("wrong coupon id ", err);}


  }

Instantiating the object for showing the coupon details when clicked also saves you space compared to when you already make object for each coupon beforehand. — binariedMe, May 01 '17 at 21:06

score 1 · Answer 2 · answered May 01 '17 at 20:53

You should create a DOM element instead of html as text and put it in a separate scope.

I have not tested this code:

for (var i = 0; i < dataLength; i++) {
   if (data.type === "coupon") {
       // separate scope
      (function(i, data) {
       var couponId = "coupon_" + i;
       // local scope
       var myCoupon =  new CouponPopup( couponId,
           data.rewards[i].prize_name,
           data.rewards[i].url,
           data.rewards[i].expirationdate);
       // create a DOM element
       var link = document.createElement('a');
       link.id = couponId;
       link.onclick = function(event) { myCoupon.show(event); };
       link.innerHTML = 'View Coupon';
       // append your link somewhere
      })(i, data);
   }
}

Imo the CouponPopup function should return this; at the end, but i might be wrong.

This looks about right, however - the link when it is rendered is now missing the onClick attribute/function. Clicking on it does nothing. Thinking its something stupid and I am over complicating thing — Waterskier19, May 02 '17 at 01:10

score 0 · Accepted Answer · answered May 02 '17 at 01:21

OK, I got it - very close to what @DimitriL sucggested - here the final code:

if (data.type === "coupon") { {
   var link = ( function( i, data  )
   {
      var couponId = "coupon_" + i;
      var myCoupon = new CouponPopup(couponId,
                  data.rewards[i].prize_name,
                  data.rewards[i].url,
                  data.rewards[i].expirationdate);

      var link = document.createElement('a');
      link.id = couponId;
      clickMe = function (event) {
                  console.log("in clickme");
                  myCoupon.show(event);
                };
      link.onclick = this.clickMe;
      link.innerHTML = "View Coupon";
      return link;
   })( i, data );

 $(reward).append(link);

}

Thanks to everyone for the help - closure and scope issues are a pain sometimes!

JavaScript scope - getting right instance of an object for onClick in loop

3 Answers3