2

I have the following code that I am trying to use to register a callback on an array of buttons. But I cannot seem to understand how I can bind the strings that I would need in the callback. Any suggestions would be much appreciated!

for (var i = 0; i < this.car_types.length; ++i) {

    this.select_car_buttons.push($("#button_select_car_" +
                this.car_types[i].car_type));

    this.select_car_buttons[this.select_car_buttons.length - 1]
        .click(function() {
            console.log(this.car_types[i].car_type);
    }.bind(this));
}

Somehow the this object is the button itself and not the object under whose scope the function is called.

EDIT : It seems like the this object was indeed being passed in properly. The issue is that the variable i is not going out of scope and is being captured by reference not by value. How should I go about solving this problem?

Also there seem to lots of such issues with JavaScript as a language (at least things that can be classified as an issue considering the semantics employed by the traditional C family languages such as C and C++ to be correct), is there some article I can read that warns me against these types of issues?

ANOTHER EDIT : On trying making a closure with the value of i captured by value I tried the following code 

this.select_car_buttons[this.select_car_buttons.length - 1]
   .click((function(scoped_i) {
      return function() {
            console.log(this.car_types[scoped_i].car_type);
      }.bind(this);                               
}(i)));

But I get the following error in Safari 

TypeError: undefined is not an object (evaluating 'scoped_i')

EDIT : The same code works in Firefox and Chrome but not in Safari!

Curious
  • 20,870
  • 8
  • 61
  • 146
  • Why don't you just avoid this kind of tricky things with `this` and on the scope object, add `self = this;`? – Miguel Lattuada Apr 10 '16 at 00:17
  • Possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – CBroe Apr 10 '16 at 00:26
  • 1
    @Curious sorry deleted my answer. I'd end up repeating what's written in the link above – Lim H. Apr 10 '16 at 00:30
  • @LimH. I still have not been able to solve my problem... `let` gives a syntax error on safari.. – Curious Apr 10 '16 at 00:38
  • @MiguelLattuada How do I achieve what you are suggesting? – Curious Apr 10 '16 at 00:39
  • self = this; before the for loop, and every time you want to access to your object use self.something, and when you want to access the scope, use this. – Miguel Lattuada Apr 10 '16 at 00:41
  • @MiguelLattuada How would that help? `self` is just a reference to `this` which is just a reference to the bound object right? – Curious Apr 10 '16 at 00:44
  • Yes, but the difference is, `self` will not be attached to the current scope that it's called. – Miguel Lattuada Apr 10 '16 at 00:46
  • @MiguelLattuada I still don't understand the benefit. Could you please elaborate? I come from a traditional C/C++/Python background so I don't know much about what javascript – Curious Apr 10 '16 at 00:47
  • Here it's a great article by Todd, https://toddmotto.com/everything-you-wanted-to-know-about-javascript-scope/, it has a great technical approach, and cover a few more topics. I hope this will help more than my random comments. `What is the this keyword and how does Scope affect it?` – Miguel Lattuada Apr 10 '16 at 00:50
  • I still cannot find a good solution to my problem – Curious Apr 10 '16 at 01:03
  • Since you're dealing with an array, just use the car_types.forEach method to encapsulate the scope, even down the road when onclick is fired. – Charlie Schliesser Apr 10 '16 at 16:36

2 Answers2

3

This is a scope issue. For modern browsers (that support ES6) you could just change var to let in your for loop and it would get fixed.

for (let i = 0; i < this.car_types.length; ++i)

Quoting the MDN docs

The let statement declares a block scope local variable, optionally initializing it to a value.

For more global support (non ES6 support) use an immediately invoked function to create extra scope for the variable (which you will pass as a parameter)

this.select_car_buttons[this.select_car_buttons.length - 1]
  .click((function(scoped_i) { // IIF starts here, the new variable is called scoped_i for verbosity
    return function() { // your original function code goes here
      console.log(this.car_types[scoped_i].car_type); // use the newly scoped variable
    }.bind(this);
  }.bind(this)(i))); // end and execute the IIF while passing the i variable to it
Gabriele Petrioli
  • 191,379
  • 34
  • 261
  • 317
0

Yes, this structure do make a lot of closures and make code very hard to read. Since you use jQuery, there are a much better way to solve this problem which saves the data in html:

html:

<button class="select-car" data-car-type="CarA">Select CarA</button>
<button class="select-car" data-car-type="CarB">Select CarB</button>
<!-- And a lot of buttons -->

js:

var selectCarOnClick = function() {
  console.info($(this).data('car-type'));
};
$('button.select-car').click(selectCarOnClick);

Live exmaple: http://codepen.io/SCLeo/pen/VaQYjW

If you have a lot of other information to store and you want to use a object to store them instead of DOM, you can save car-name or car-id instead of car-type.

Here is the document about $.data: https://api.jquery.com/jquery.data/

SCLeo
  • 353
  • 3
  • 14