2

I'm just studying javascript and I faced an issue related to scoping.

Here's the code

function User (properties) {
    for (var i in properties) {
        (function ()  {
            this ['get' + i] = function () {
                return properties [i];
            };
        }) ();
    }
}

var me = new User ({
    Id : 54,
    Name : 'ohyou'
});

console.log (me.getName ());
console.log (me.getId ());

How I want it to work: it should create two functions getName and getId that belong to the me object.

How it works: it creates two functions just as I want, but they belong to the window

What I tried:

  • I tried removing the function on the line 3. It does what I want, but now it returns the name "ohyou" two times, instead of returning the id and the name

  • I tried using the functions from the window scope, the problem as in the previous case persists - it returns the name twice.

The question: what am I doing wrong and how do I make it work as I want?

P.S. Making this post using phone, sorry for possible typos or formatting issues. Also, please forgive me my english skills.

keke
  • 105
  • 2
  • 2
  • 14
  • This is probably a duplicate of a duplicate somewhere. Best solution IMO is to drop the loop, and use `Object.keys(properties).forEach`, – elclanrs Oct 10 '14 at 06:22
  • 1
    You can deal with `this` or you can deal with `that`. http://stackoverflow.com/questions/4886632/what-does-var-that-this-mean-in-javascript – Paul Oct 10 '14 at 06:27
  • Changed the loop to the function you suggested, but, apparently, it didn't work. – keke Oct 10 '14 at 06:27

4 Answers4

2

Try to understand why the following code works:

function User(properties) {
    for (var i in properties) {
        with ({ i: i, self: this, props: properties }) {
            self["get" + i] = function () {
                return props[i];
            };
        }
    }
}

var me = new User({
    Id: 54,
    Name: "ohyou"
});

alert(me.getName());
alert(me.getId());

This is an example of one of the legitimate uses of the with statement.

I find the with statement to be more succinct than using an immediately invoked function expression (IIFE) as others are suggesting.

Edit: The with keyword is not bad if you use it correctly. There are some legitimate uses of with:

http://webreflection.blogspot.in/2009/12/with-worlds-most-misunderstood.html


The reason your code doesn't work is because every function has it's own value of this. Hence when you immediately invoke the function expression within the for loop, the value of this inside the function is no longer your new object. It is window. To solve that problem you could do something like:

(function (i, self, props) {
    self["get" + i] = function () {
        return props[i];
    };
}(i, this, properties))

However using the with statement is clean and faster. It's faster because you're not calling a function.

Aadit M Shah
  • 72,912
  • 30
  • 168
  • 299
  • Hey Aadit - I personally like the usage of 'with' here, but the docs I've read shun away from it - though I believe your usage here is nice. – james emanon Oct 10 '14 at 06:32
  • 1
    @jamesemanon The `with` statement is not bad. See the following question: http://stackoverflow.com/q/1931186/783743 – Aadit M Shah Oct 10 '14 at 06:35
  • Thank you very much. It works perfectly even with just `{i: i}`, but I don't understand why. And I also don't understand why it didn't work in my case. – keke Oct 10 '14 at 06:36
  • @ohyou I added an explanation as to why your code doesn't work as expected. – Aadit M Shah Oct 10 '14 at 06:41
2
function User (properties) {
    var that = this;
    for (var i in properties) {
        (function (i)  {

            that ['get' + i] = function () {
                return properties [i];
            };

        }) (i);
    }
}

var me = new User ({
    Id : 54,
    Name : 'ohyou'
});
james emanon
  • 11,185
  • 11
  • 56
  • 97
2

You can avoid all these closures and other things if you use Object.keys and then forEach, which already introduces a new scope. Then you'd have to pass the this value, which is the second parameter:

function User(properties) {
  Object.keys(properties).forEach(function(k) {
    this['get'+ k] = function() {
      return properties[k]
    }
  },this)
}
elclanrs
  • 92,861
  • 21
  • 134
  • 171
  • Although this is terribly slow. First loop through all the properties to get the key names. Then loop through them again, running the callback function each time. – Aadit M Shah Oct 10 '14 at 06:36
  • Nothing to worry about, engines are plenty fast. I'd stay away from `for` loops if I want readable code, and better abstractions. – elclanrs Oct 10 '14 at 06:38
  • I guess everybody has their own style. I like writing functional code too, but not in JavaScript. Anyway, +1. – Aadit M Shah Oct 10 '14 at 06:43
0

One thing you must remember, any function that does not belong to any object, will always belong to window object.

For example if we modify your object and add a new method,

me.doSomeWork = function(){
   this.myLuckyNumber = 10;
   var that = this;
   function doubleMyLuckyNumber(){
      console.log(this); //window
      that.myLuckyNumber = that.myLuckyNumber * 2;     
   } 

  doubleMyLuckyNumber();  
};

me.doSomeWork();
console.log(me.myLuckyNumber) //outputs 20

So always save the reference to a var to use it in inner methods. You can use any of the way that others suggested but I prefer James emanon's approach.

Sohel
  • 431
  • 3
  • 10
  • I knew that, thanks for reminding. I just got confused with the inner scope I didn't pass my data to. – keke Oct 10 '14 at 08:00