6

I'm trying to solve this puzzle minded Javascript OOP problem.

So I have the following class :

var ClassA = function() {
    this.initialize();
}

ClassA.prototype = {

    methods : ['alpha','beta','gama'],

    initialize : function() {
        for ( var i in this.methods ) {
            this[this.methods[i]] = function() {
                console.log(this.methods[i]);
            }
        }
    }
}

var a = new ClassA();

When I call every method I expect to print the name of it, right? But here is what i get :

a.alpha(); // returns gama ?!?
a.beta();  // returns gama ?!?
a.gama();  // returns gama

But when my class looks like this :

var ClassB = function() {
    this.initialize();
}

ClassB.prototype = {

    methods : ['alpha', 'beta', 'gama'],

    initialize: function() {
        for ( var i in this.methods ) {
            this.addMethod(this.methods[i]);
        }
    },

    addMethod: function(method) {
        this[method] = function() {
            console.log(method);
        }
    }

}

var b = new ClassB();

b.alpha(); // returns alpha
b.beta();  // returns beta
b.gama();  // returns gama

Why is this happening ?

drinchev
  • 19,201
  • 4
  • 67
  • 93
  • Isn't that [the incorrect way](http://stackoverflow.com/questions/3010840/loop-through-array-in-javascript#answer-3010848) of looping through an array in JS? – PeeHaa Apr 02 '12 at 21:36
  • @RepWhoringPeeHaa - Yes, a plain for loop should be used, but that isn't the problem here. – nnnnnn Apr 02 '12 at 21:48
  • @nnnnnn I know that's why it is a comment and not an answer :-) – PeeHaa Apr 02 '12 at 21:51

4 Answers4

6
for ( var i in this.methods ) {
      this[this.methods[i]] = function() {
          console.log(this.methods[i]);
       }
}

Your problem lies here. When this loop ends, i is the last element. Each function uses the same i, so they are all the last element.

When you use addMethod you are making a closure to "capture" the correct value.

EDIT: When you call addMethod you are "copying" the value, instead of using the i value, which changes with each loop iteration.

gen_Eric
  • 223,194
  • 41
  • 299
  • 337
  • I answer a question like this almost every day... http://stackoverflow.com/questions/9980209/register-onclick-events-from-dynamically-created-div-array-rails-jquery/9980579#9980579 – Ruan Mendes Apr 02 '12 at 21:36
  • I'm confused... Isn't console.log just another function, and by addMethod I'm just wrapping it around into another one? – drinchev Apr 02 '12 at 21:38
  • @drinchev: `console.log` isn't important here. What's important here, is that when you call `addMethod`, you're copying the value into each function, but when you do it without it, you're using the same value. (Sorry if I'm not explaining this well.) – gen_Eric Apr 02 '12 at 21:40
  • The first example is a closure because it makes a new scope that references something in its enclosing scope. The second one copies the value during the function call, so it's not a closure. http://en.wikipedia.org/wiki/Closure_(computer_science) – dtanders Apr 02 '12 at 21:49
  • @dtanders: But there is a closure inside of the `addMethod` function :-P – gen_Eric Apr 02 '12 at 21:50
  • @Rocket Ah, now I see what you were getting at - I was only thinking about the loop code. – dtanders Apr 02 '12 at 21:56
  • @Rocket, thank you for your help. I have a last question ... So you say that when I call addMethod I'm copying the value, but doesn't console.log copy the value too, because it acts like closure ( it is a function, right )? – drinchev Apr 02 '12 at 22:05
  • @drinchev: `console.log` isn't ran until you actually call `a.alpha`. Yes, it would get a copy, we're just making sure the copy it gets is the correct value. (I don't know if that makes sense.) – gen_Eric Apr 02 '12 at 22:07
3

In your first version:

initialize : function() {
    for ( var i in this.methods ) {
        this[this.methods[i]] = function() {
            console.log(this.methods[i]);
        }
    }
}

The methods that you create within initialize all refer to the same i variable from initialize - and after initialize runs i has the value "gama", so regardless of which of the methods you call that's the value of i that they'll log to the console. JS doesn't store the current value of i at the time the method is created.

JS creates a "closure" for each function - variables declared in your initialize function (i.e., i) continue to be in scope for the nested function(s) even after initialize has finished.

The second version calls addMethod to add each method:

addMethod: function(method) {
    this[method] = function() {
        console.log(method);
    }
}

...and so when they run they'll refer to their own "copy" of the method parameter because then there is a separate closure for each of the methods.

Edit: See also this question: How do JavaScript closures work? (several answers there explain this more clearly than I did).

Community
  • 1
  • 1
nnnnnn
  • 147,572
  • 30
  • 200
  • 241
1

You can fix your first example by adding an anonymous closure:

initialize : function() {
    for ( var i in this.methods ) {
        (function (i) { // anonymous closure
            this[this.methods[i]] = function() {
                console.log(this.methods[i]);
            }
        }).call(this, i); // use .call() if you need "this" inside
    }
}

Now it will work the same way as your second example. "Anonymous" means that the closure is made by function which doesn't have a name and is called instantly as it is "created".

Note sideways: use .call(this, ...) to preserve this inside the called function, or you can do var that = this, use that instead of this and call the function normally:

for ( var i in this.methods ) {
    var that = this;
    (function (i) { // anonymous closure 
        that[that.methods[i]] = function() {
            console.log(that.methods[i]);
        }
    })(i); // Called normally so use "that" instead of "this"!
}
Tomas
  • 57,621
  • 49
  • 238
  • 373
0

Well, first of all stop using for (property in object) loops on Arrays. It's all fun and games until somebody prototypes to the Array object which is both a perfectly reasonable and very useful/popular thing to do. This will result in custom methods getting added to your for x in array loops.

As for the problem, it's doing exactly what you told it to do in version 1. The problem is that by the time you get around to firing it, i is the last thing i was, 'gamma'. When you pass a reference into a function as an argument, the function holds on to the value's state as it was passed.

Erik Reppen
  • 4,605
  • 1
  • 22
  • 26