35

As you know we can define getters and setters in JS using defineProperty(). I've been stuck when trying to extend my class using defineProperty().

Here is an example code:

I have an array of fields which must be added to a object

fields = ["id", "name", "last_login"]

Also I have a class which will be modified

var User = (function(){
    // constructor
    function User(id, name){
        this.id     = id
        this.name   = name
    }
    return User;
})();

And a function which will add fields to the class using defineProperty()

var define_fields = function (fields){
    fields.forEach(function(field_name){
        var value = null
        Object.defineProperty(User.prototype, field_name, {
            get: function(){ return value }
            set: function(new_value){
                /* some business logic goes here */
                value = new_value
            }
        })
    })
};

After running define_fields() I have my fields in the instance of the User

define_fields(fields);
user1 = new User(1, "Thomas")
user2 = new User(2, "John")

But the values ​​of these properties are identical

console.log(user2.id, user2.name) // 2, John
console.log(user1.id, user1.name) // 2, John

Is there any way to make defineProperty() work properly in this case? If I understand the problem is with value which becomes identical for each instance of the class but i can't realise how to fix it. Thanks in advance for your answers.

UPD: This way throws "RangeError: Maximum call stack size exceeded"

var define_fields = function (fields){
    fields.forEach(function(field_name){
        Object.defineProperty(User.prototype, field_name, {
            get: function(){ return this[field_name] }
            set: function(new_value){
                /* some business logic goes here */
                this[field_name] = new_value
            }
        })
    })
};
nick.skriabin
  • 873
  • 2
  • 11
  • 27
  • 3
    Gotta love JavaScript. One moment you think you got the hang of all the things you can express with it, and then it proves you horribly horrible wrong. – Dmytro Aug 24 '16 at 12:40
  • 1
    Gotta be scared of js in production. Will never accept a job to fix a js application I did not write! – NoChance Oct 05 '17 at 11:15

6 Answers6

58

Please don't implement any other version because it will eat all your memory in your app:

var Player = function(){this.__gold = 0};

Player.prototype = {

    get gold(){
        return this.__gold * 2;
    },



    set gold(gold){
        this.__gold = gold;
    },
};

var p = new Player();
p.gold = 2;
alert(p.gold); // 4

If 10000 objects are instantiated:

  • With my method: you will only have 2 functions in the memory;
  • With the other methods: 10000 * 2 = 20000 functions in the memory;
ShAkKiR
  • 863
  • 9
  • 20
Totty.js
  • 15,563
  • 31
  • 103
  • 175
18

I came to the same conclusion as Mikhail Kraynov three minutes after he answered. That solution defines new properties each time the constructor is called. I wondered if, as you asked, there was a way of putting the getters and setters in the prototype. Here is what I came up with:

var User = (function () {
  function User (id, nam) {
    Object.defineProperty (this, '__',  // Define property for field values   
       { value: {} });

    this.id = id;
    this.nam = nam;
  }

  (function define_fields (fields){
    fields.forEach (function (field_name) {
      Object.defineProperty (User.prototype, field_name, {
        get: function () { return this.__ [field_name]; },
        set: function (new_value) {
               // some business logic goes here 
               this.__[field_name] = new_value;
             }
      });
    });
  }) (fields);

  return User;
}) ();  

In this solution I define the field getters and setters in the prototype but reference a (hidden) property in each instance which holds the field values.

See the fiddle here : http://jsfiddle.net/Ca7yq

I added some more code to the fiddle to show some effects on enumeration of properties : http://jsfiddle.net/Ca7yq/1/

Community
  • 1
  • 1
HBP
  • 15,685
  • 6
  • 28
  • 34
  • You're welcome. This does have the curious effect that objects with the same constructor do not necessarily have any properties in common. An interesting fact which must surely have an awesome application somewhere – HBP Dec 27 '12 at 13:30
  • I'll keep my app in secret :) Later it will be published in Chrome Web Store and maybe some of it's source codes will be on GitHub. – nick.skriabin Dec 27 '12 at 13:52
  • This has the strange behavior of circular references among instances of this object when you use multiple. You cannot `JSON.Stringify()` these instances because it will throw `TypeError: Converting circular structure to JSON`. I am not sure why. – Redsandro Jun 19 '13 at 13:37
  • 3
    Great Solution! I just wanted to add that you can add `User.prototype.toJSON = function() { return this.__; }` inside of your `User` constructor so that you can `JSON.stringify(user1)` directly instead of having to `JSON.stringify(user1.__)`. If you are using a persistence layer that Stringifies the object, you might then be able to just pass off the object to a save method! – Kyle Mueller Oct 11 '13 at 15:31
  • 1
    [This article](http://www.2ality.com/2016/01/private-data-classes.html) is written for a similar context (ES6 Classes). The WeakMap approach would fit well here. – ashwell Jan 26 '16 at 08:48
8

It seems to me, that when you defineProperties for prototype, all instances shares that properties. So the right variant could be

var User = (function(){
// constructor
function User(id, name){
    this.id     = id
    this.name   = name

    Object.defineProperty(this, "name", {
        get: function(){ return name },
        set: function(new_value){
            //Some business logic, upperCase, for example
            new_value = new_value.toUpperCase();
            name = new_value
        }
    })
}
return User;
})();
  • 2
    Yeah, i thought about this solution. But it will bring some other problems. Let's say I have class `Model` which is abstract and a `User` which is inherited of `Model`. So i need something which will automatically create needed methods specifically for `User` class. I don't want write definitions of these methods each time when I create new model. For example little later I'll have classes `Album` and `Song` which will be inherited of `Model` too. – nick.skriabin Dec 27 '12 at 11:34
  • 3
    Wouldn't this eat up all your memory? It will create new functions for each instance. – Totty.js Oct 03 '14 at 14:43
5

As you define your properties on the prototype object of all user instances, all those objects will share the same value variable. If that is not what you want, you will need to call defineFields on each user instance separately - in the constructor:

function User(id, name){
    this.define_fields(["name", "id"]);
    this.id     = id
    this.name   = name
}
User.prototype.define_fields = function(fields) {
    var user = this;
    fields.forEach(function(field_name) {
        var value;
        Object.defineProperty(user, field_name, {
            get: function(){ return value; },
            set: function(new_value){
                /* some business logic goes here */
                value = new_value;
            }
        });
    });
};
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Is this sharing thing apart of the ES spec? Before I figured this out I was very confused. – pllee Oct 27 '14 at 22:12
  • 1
    Yes, the behaviour is specified in ES5 (combining the sections about prototypes, closures, etc) – Bergi Oct 27 '14 at 23:11
1

This solution is without extra memory consumption. Your updated code is close. You just need to use this.props[field_name] instead of direct this[field_name].

Please note that defineProperty call replaced to Object.create

Js Fiddle http://jsfiddle.net/amuzalevskyi/65hnpad8/

// util
function createFieldDeclaration(fields) {
    var decl = {};
    for (var i = 0; i < fields.length; i++) {
        (function(fieldName) {
            decl[fieldName] = {
                get: function () {
                    return this.props[fieldName];
                },
                set: function (value) {
                    this.props[fieldName] = value;
                }
            }
        })(fields[i]);
    }
    return decl;
}

// class definition
function User(id, name) {
    this.props = {};
    this.id = id;
    this.name = name;
}
User.prototype = Object.create(Object.prototype, createFieldDeclaration(['id','name']));

// tests
var Alex = new User(0, 'Alex'),
    Andrey = new User(1, 'Andrey');

document.write(Alex.name + '<br/>'); // Alex
document.write(Andrey.name + '<br/>'); // Andrey

Alex.name = "Alexander";
document.write(Alex.name + '<br/>'); // Alexander
document.write(Andrey.name + '<br/>'); //Andrey
Andrii Muzalevskyi
  • 3,261
  • 16
  • 20
0

From the accepted answer, I realize what we're trying to do here is defining private instance variables. These variables should be on the instance (this), rather than on the prototype object. Normally we name private variables by prefixing an underscore to the property's name.

var Vehicle = {};
Object.defineProperty(Vehicle, "make", {
    get: function() { return this._make; }
    set: function(value) { this._make = value; }
});

function Car(m) { this.make = m; }    //this will set the private var _make
Car.prototype = Vehicle;

The accepted answer basically puts all private variables in a container instead, which is actually better.

Sarsaparilla
  • 6,300
  • 1
  • 32
  • 21
  • You're totally right. Local (private) variables inside the scope is more efficient and secure way. But there is another question: how to define variables dynamically? I want to have only one class, for example `Model` on the client and then i'll do something like `MyCustomModel extends Model` and the `Model` will handle properties definition on particular class. You can take Rails ActiveModel as reference of what i want to do. – nick.skriabin Oct 22 '14 at 14:33
  • And `eval` is not a good way to dynamically define variables :) – nick.skriabin Oct 22 '14 at 14:36
  • I think you're trying to do some Javascript acrobatics :). For local variables I think we have no options but to use eval. In one of my programs I do: var expr=""; for (var name in vars) expr+="var "+name+"=vars[name];\n"; eval(expr); ...use the local vars. Not sure how to use it in an inheritance context though – Sarsaparilla Oct 23 '14 at 02:20