At the moment I am using the following code to define getter and setters for my class:

    Object.defineProperty(FPProject.prototype, 'name', {
        get: function() { return this.get('name'); },
        set: function(aValue) {return this.set('name', aValue);}
    });
    Object.defineProperty(FPProject.prototype, 'code', {
        get: function() { return this.get('code'); },
        set: function(aValue) {return this.set('code', aValue);}
    });
    Object.defineProperty(FPProject.prototype, 'clientName', {
        get: function() { return this.get('clientName'); },
        set: function(aValue) {return this.set('clientName', aValue);}
    });
    Object.defineProperty(FPProject.prototype, 'client', {
        get: function() { return this.get('client'); },
        set: function(aValue) {return this.set('client', aValue);}
    })

I thought I could optimise this code to something like this :

    var fields = ['name','code','clientName','client'];

    for (var i = 0; i < fields.length; i ++ ) {
        Object.defineProperty(FPProject.prototype, fields[i], {
            get: function() { return this.get(fields[i]); },
            set: function(aValue) {return this.set(fields[i], aValue);}
        });
    }

but it doesn't work! I get no console errors, just cant set the properties... ???

At the moment I am using the following code to define getter and setters for my class:

    Object.defineProperty(FPProject.prototype, 'name', {
        get: function() { return this.get('name'); },
        set: function(aValue) {return this.set('name', aValue);}
    });
    Object.defineProperty(FPProject.prototype, 'code', {
        get: function() { return this.get('code'); },
        set: function(aValue) {return this.set('code', aValue);}
    });
    Object.defineProperty(FPProject.prototype, 'clientName', {
        get: function() { return this.get('clientName'); },
        set: function(aValue) {return this.set('clientName', aValue);}
    });
    Object.defineProperty(FPProject.prototype, 'client', {
        get: function() { return this.get('client'); },
        set: function(aValue) {return this.set('client', aValue);}
    })

I thought I could optimise this code to something like this :

    var fields = ['name','code','clientName','client'];

    for (var i = 0; i < fields.length; i ++ ) {
        Object.defineProperty(FPProject.prototype, fields[i], {
            get: function() { return this.get(fields[i]); },
            set: function(aValue) {return this.set(fields[i], aValue);}
        });
    }

but it doesn't work! I get no console errors, just cant set the properties... ???

• javascript

Share Follow edited Apr 26, 2014 at 8:28 a better oliver 26.9k22 gold badges6464 silver badges6666 bronze badges asked Apr 26, 2014 at 8:21 simonberrysimonberry 93099 silver badges2020 bronze badges

Add a ment  | 

4 Answers 4

Sorted by: Reset to default 4

The other two answers work just fine (and I upvoted them), but I thought I'd add that this is one place where an array iterator method es in handy because the very task of using it creates a function closure for your operation which solves this index problem for you automatically:

['name','code','clientName','client'].forEach(function(item) {
    Object.defineProperty(FPProject.prototype, item, {
        get: function() { return this.get(item); },
        set: function(aValue) {return this.set(item, aValue);}
    });
});

And, of course, you have to pay attention to browser patibility of .forEach() which requires IE9 or greater or a polyfill (shown here).

FYI, this design pattern is used by a popular third party library because it's a very pact way of running the same code with several different values run through it.

When your loop finishes, the i variable will be equal to fields.length, your set function called later will use this value.

Try using closure to capture the current index element:

for (var i = 0; i < fields.length; i ++ ) {
    (function (index) {
          Object.defineProperty(FPProject.prototype, fields[index], {
            get: function() { return this.get(fields[index]); },
            set: function(aValue) {return this.set(fields[index], aValue);}
          });
    }(i));
}

This is the classic closure problem. The functions you create in your loop have an enduring reference to the i variable, not a copy of it. So by the time your accessor functions are called, i is fields.length and so the value they get from fields[i] is undefined.

The usual solution is a builder function:

var fields = ['name','code','clientName','client'];

for (var i = 0; i < fields.length; i ++ ) {
    buildProperty(FPProject.prototype, fields[i]);
}

function buildProperty(obj, name) {
    Object.defineProperty(obj, name, {
       get: function() { return this.get(name); },
       set: function(aValue) {return this.set(name, aValue);}
    });
}

I always make this a nice, clear, separate function so that A) We're not recreating it on every loop, and B) It's easier to debug, understand, and reuse.

Now the accessors close over the context of the call to buildProperty and its obj and name arguments, which don't change, and so when they're called they use the correct name.

Faced same problem, solved by just adding this

Object.defineProperty(FPProject.prototype, 'name', {
    get: function() { return this.get('name'); },
    set: function(aValue) {return this.set('name', aValue);},
    enumerable: true, // <--
    configurable: true // <--
});

https://developer.mozilla/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty#description