20

I'm reading "Pro JavaScript Techniques" by John Resig, and I'm confused with an example. This is the code:

// Create a new user object that accepts an object of properties
function User( properties ) {
  // Iterate through the properties of the object, and make sure
  // that it's properly scoped (as discussed previously)
  for ( var i in properties ) { (function(){
  // Create a new getter for the property
  this[ "get" + i ] = function() {
    return properties[i];
  };
  // Create a new setter for the property
  this[ "set" + i ] = function(val) {
    properties[i] = val;
  };
})(); }
}

// Create a new user object instance and pass in an object of
// properties to seed it with
var user = new User({
  name: "Bob",
  age: 44
});

// Just note that the name property does not exist, as it's private
// within the properties object
alert( user.name == null );

// However, we're able to access its value using the new getname()
// method, that was dynamically generated
alert( user.getname() == "Bob" );

// Finally, we can see that it's possible to set and get the age using
// the newly generated functions
user.setage( 22 );
alert( user.getage() == 22 );

Now running that in the Firebug console (on Firefox 3) throws that user.getname() is not a function. I tried doing this:

var other = User
other()
window.getname() // --> This works!

And it worked!

Why?

Doing:

var me = this;

seems to work a bit better, but when executing "getname()" it returns '44' (the second property)...

Also I find it strange that it worked on the window object without modification...

And a third question, what's the difference between PEZ solution and the original? (He doesn't use an anonymous function.)

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Pablo Fernandez
  • 103,170
  • 56
  • 192
  • 232
  • I generally try to make code as simple as it can be. Thus I removed the anonymous function stuff where I didn't think it was needed. – PEZ Dec 19 '08 at 16:22

8 Answers8

5

I think it's best not to use the new keyword at all when working in JavaScript.

This is because if you then instantiate the object without using the new keyword (ex: var user = User()) by mistake, *very bad things will happen...*reason being that in the function (if instantiated without the new keyword), the this will refer to the global object, ie the window...

So therefore, I suggest a better way on how to use class-like objects.

Consider the following example :

var user = function (props) {
    var pObject = {};
    for (p in props) {
        (function (pc) {
            pObject['set' + pc] = function (v) {
                props[pc] = v;
                return pObject;
            }
            pObject['get' + pc] = function () {
                return props[pc];
            }
        })(p);
    }
    return pObject;
}

In the above example, I am creating a new object inside of the function, and then attaching getters and setters to this newly created object.

Finally, I am returning this newly created object. Note that the the this keyword is not used anywhere

Then, to 'instantiate' a user, I would do the following:

var john = user({name : 'Andreas', age : 21});
john.getname(); //returns 'Andreas'
john.setage(19).getage(); //returns 19

The best way to avoid falling into pitfalls is by not creating them in the first place...In the above example, I am avoiding the new keyword pitfall (as i said, not using the new keyword when it's supposed to be used will cause bad things to happen) by not using new at all.

Andreas Grech
  • 105,982
  • 98
  • 297
  • 360
  • 2
    `new` is not evil if handled correctly, as [seen here](http://stackoverflow.com/questions/383402/is-javascripts-new-keyword-considered-harmful) – Adowrath Nov 10 '16 at 09:15
  • I've never really had this problem, as I never accidentally forget to use new. I use naming conventions so I 100% know when I'm dealing with a class and when I'm not, i.e "let user = new adUserClass(apiData.rawUser);" If it doesn't have Class on it's name in an import etc, don't use new, pretty simple. – Ryan Mann Jan 19 '21 at 01:26
4

Adapting Jason's answer, it works:

We need to make a closure for the values. Here's one way:

function bindAccessors(o, property, value) {
  var _value = value;
  o["get" + property] = function() {
    return _value;
  };
  o["set" + property] = function(v) {
    _value = v;
  };
}

Then the User constructor looks like this:

function User( properties ) {
  for (var i in properties ) {
    bindAccessors(this, i, properties[i]);
  }
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
PEZ
  • 16,821
  • 7
  • 45
  • 66
  • but calling the function on the window object(as I stated later) does work, I don't think that loop is the problem – Pablo Fernandez Dec 18 '08 at 13:01
  • Note to readers; Pablo is right. My answer is edited to reflect that. – PEZ Dec 18 '08 at 13:07
  • Two comments: 1) thanks for the attribution (would be clearer to use my full username), now maybe someone could vote for my answer....? 8) 2) One change you made is very subtle: The getters and setters do NOT modify the properties hash, but rather the set of its values... (cont'd) – Jason S Dec 19 '08 at 14:19
  • 2 cont'd) I can pass in a collection to the User constructor and the collection itself will NOT change. If that is a desired feature then you need to reference the properties array in the closure. – Jason S Dec 19 '08 at 14:20
  • +1 for your answer. (I'd add +3 for pointing out the caveats with my answer if I could.) – PEZ Dec 19 '08 at 14:58
3

You probably want something like this, which is more readable (closures are easy to learn once you get some practice):

function User( properties ) {
  // Helper function to create closures based on passed-in arguments:
  var bindGetterSetter = function(obj, p, properties)
  {
    obj["get" + p] = function() { return properties[p]; }
    obj["set" + p] = function(val) { properties[p]=val; return this; }
  };
  for (var p in properties)
    bindGetterSetter(this, p, properties);
}

I also added "return this;", so you can do:

u = new User({a: 1, b:77, c:48});
u.seta(3).setb(20).setc(400)
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Jason S
  • 184,598
  • 164
  • 608
  • 970
3

I started this post with the sole purpose of learning why that things happened, and I finally did. So in case there's someone else interested in the "whys", here they are:

Why does 'this' changes inside the anonymous function?

A new function, even if it is an anonymous, declared inside an object or another function, always changes the scope, in this case returning to the global scope (window).

Solution: all stated in the post, I think the clearer is executing the anonymous function with .call(this).

Why does getname() always return the age?

While the anonymous function gets executed right away, the getters/setters get executed for the first time when they are called. In that moment, the value of i will always be the last, because it has already iterated for all the properties... and it will always return properties[i] which is the last value, in this case the age.

Solution: save the i value in a variable like this

 for ( i in properties ) { (function(){ 
  var j = i
  // From now on, use properties[j]
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Pablo Fernandez
  • 103,170
  • 56
  • 192
  • 232
  • "Solution: save the i value in a variable like this" (then "var j = i") Be careful! This is misleading. I'm not sure whether or not it will work. What a closure basically does is to hold on to a reference to a particular variable from outside the function. (cont'd...) – Jason S Dec 19 '08 at 14:24
  • (contd) You'll (usually) want separate closures to each refer to their own instances of a variable they are referring to. There is only one variable "i" in the for loop. I'm not sure whether "j" gets a new instance each loop iteration, or whether Javascript reuses it (in which case your soln fails) – Jason S Dec 19 '08 at 14:27
  • The solution works. I've tested it and it's the only one that does what I've asked in the firs place. – Pablo Fernandez Dec 19 '08 at 15:16
  • interesting! then that says "j" gets a new instance each loop iteration. (at least for the implementation you're using, not sure if that's in the Javascript/ECMAScript spec, so just a warning as that might be implementation-dependent behavior e.g. could be different on different browsers) – Jason S Dec 19 '08 at 16:48
  • Hmm, I repeat my warning. Here's a test that runs in Rhino or the Mozilla Spidermonkey Javascript shell: var x = []; for (var i = 0; i < 10; ++i) { var j = i; x.push(function() { return j; } ); } x[0]() and x[9]() both return 9. (not 0 and 9) – Jason S Dec 19 '08 at 16:54
  • 1
    OH, NEVER MIND! You are copying "var j = i" within the anonymous function that executes. That definitely creates a new instance. Javascript has function scope, not block scope. – Jason S Dec 19 '08 at 16:56
2

As written in the OP, this in the loop is not referring to the User object as it should be. If you capture that variable outside the loop, you can make it work:

function User( properties ) {
  // Iterate through the properties of the object, and make sure
  // that it's properly scoped (as discussed previously)
 var me = this;
 for ( i in properties ) { (function(){
  // Create a new getter for the property
  me[ "get" + i ] = function() {
    return properties[i];
  };
  // Create a new setter for the property
  me[ "set" + i ] = function(val) {
    properties[i] = val;
  };
 // etc
J Cooper
  • 16,891
  • 12
  • 65
  • 110
2

I just modified the code a bit like this.. This one should work.. This is same as setting me=this; But a closure is required to set the value of each property properly, else the last value will be assigned to all properties.

    // Create a new user object that accepts an object of properties
    var User = function( properties ) {
      // Iterate through the properties of the object, and make sure
      // that it's properly scoped (as discussed previously)
      var THIS = this;
      for ( var i in properties ) { (function(i){
      // Create a new getter for the property
      THIS[ "get" + i ] = function() {
        return properties[i];
      };
      // Create a new setter for the property
      THIS[ "set" + i ] = function(val) {
        properties[i] = val;
      };
    })(i); }
    }

    // Create a new user object instance and pass in an object of
    // properties to seed it with
    var user = new User({
      name: "Bob",
      age: 44
    });

// Just note that the name property does not exist, as it's private
// within the properties object
alert( user.name == null );

// However, we're able to access its value using the new getname()
// method, that was dynamically generated
alert( user.getname() == "Bob" );

// Finally, we can see that it's possible to set and get the age using
// the newly generated functions
user.setage( 22 );
alert( user.getage() == 22 );
Shidhin Cr
  • 868
  • 8
  • 19
1

Maybe the variable i is "closured" with the last value in the iteration ("age")? Then all getters and setters will access properties["age"].

PEZ
  • 16,821
  • 7
  • 45
  • 66
0

I found something that seems to be the answer; it’s all about context. Using the anonymous function inside the for loop, changes the context, making 'this' refer to the window object. Strange isn't it?

So:

function User(properties) {

  for (var i in properties) {

    // Here this == User Object
    (function(){
      // Inside this anonymous function, this == window object
      this["get" + i] = function() {
        return properties[i];
      };

      this["set" + i] = function(val) {
        properties[i] = val;
      };
    })();
  }
}

I don't know why that function changes the context of execution, and I'm not sure it should do that. Anyway, you can test it running the code there and trying window.getname(). It magically works! :S

The solution, as stated before, is changing the context. It can be done like J Cooper said, passing the variable 'me' and making the function a closure or you can do this:

(function(){
    // Inside this anonymous function this == User
    // because we called it with 'call'
    this[ "get" + i ] = function() {
      return properties[i];
    };

    this["set" + i] = function(val) {
      properties[i] = val;
    };
 }).call(this);

Anyway, I'm still getting 44 when running 'getname'... What could it be?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Pablo Fernandez
  • 103,170
  • 56
  • 192
  • 232