Paul Aldred-Bann
Paul Aldred-Bann

Reputation: 6020

Overriding setter on object

I have an Object with a getter and setter where I'm intending to protect a property so that it can only be updated through a method. Let's call this object Person which has the following structure:

function Person(data) {
    var _name = data.name;

    this.name = function () {
        return _name;
    };
    this.setName = data.setName || function (newValue) {
        _name = newValue;
    };
}

I want to be able to override setName and pass different implementations to each instance of Person but I can't quite seem to get it to work. I'm certain this is a closure issue, but I just can't get my head around it.

Given the following usage scenario, what am I doing wrong?

var p1 = new Person({
    name: "Paul",
    setName: function (newValue) {
        this._name = newValue;
    }
});
var p2 = new Person({ name: "Bob" });
p1.setName("Paul (updated)");
p2.setName("Bob (updated)");

p1 never updates its value and so is always "Paul" whereas p2 does and becomes "Bob (updated)". I want to be able to create as many unique instances of Person as I want, where some of them have their own implementation of setName and others will just use the default instance.

I've tried wrapping data.setName up in a closure like this and setting my custom setName to return the value instead:

this.setName = data.setName ? function () {
    return (function (value) {
        value = data.setName();
    }(_name));
} : function (newValue) { _name = newValue; }

But I'm having no luck - I obviously just don't get closures as well as I thought! Any and all help always appreciated.

codepen.io example here

Upvotes: 1

Views: 507

Answers (4)

phylax
phylax

Reputation: 2086

U think it's a problem with this

this.setName = function (newValue) {
  if (data.setName) {
    data.setName.call(this, newValue);
  } else {
    _name = newValue;
  }
}

Upvotes: 0

Bergi
Bergi

Reputation: 664503

this._name = newValue;

_name is a private variable, not a property. It cannot be accessed on this. Have a look at Javascript: Do I need to put this.var for every variable in an object? if you're not sure about the difference.

function (value) {
    value = data.setName();
}

value here is a local variable to that function's scope, not a "reference" to the _name variable you passed in. Setting it will only put a different value in that local variable, but not change anything else.


Possible solutions:

  • Pass a setter function that has access to the _name variable to the validator:

    function Person(data) {
        var _name = data.name;
    
        this.name = function () {
            return _name;
        };
        if (data.nameValidator) 
            this.setName = function(newValue) {
                _name = data.nameValidator(newValue, _name);
            };
        else
            this.setName = function (newValue) {
                _name = newValue;
            };
    }
    var p1 = new Person({
        name: "Paul",
        nameValidator: function (newValue, oldValue) {
            return (newValue /* ... */) ? newValue : oldValue;
        }
    });
    
  • Or let the validator return the value:

    function Person(data) {
        var _name = data.name;
    
        this.name = function () {
            return _name;
        };
        this.setName = function (newValue) {
            _name = newValue;
        };
        if (data.makeNameValidator) 
            this.setName = data.makeNameValidator(this.setName);
    }
    var p1 = new Person({
        name: "Paul",
        makeNameValidator: function (nameSetter) {
            return function(newValue) {
                if (newValue) // ...
                    nameSetter(newValue);
            };
        }
    });
    
  • Or make _name a property to which the validator can write. You could also put this on the original data object, for not exposing it on the Person interface:

    function Person(data) {
        this.name = function () {
            return data.name;
        };
        if (data.setName)
            this.setName = data.setName.bind(data);
        else
            this.setName = function(newValue) {
                _name = newValue;
            };
    }
    var p1 = new Person({
        name: "Paul",
        setName: function (newValue) {
            if (newValue) // ...
                this.name = newValue;
        }
    });
    

Upvotes: 2

Mosho
Mosho

Reputation: 7078

I feel this is the simplest solution so far, using bind:

function Person(data) {
    var _data = {name:data.name};

    this.name = function () {
        return _data.name;
    };
    this.setName = data.setName ? data.setName.bind(_data) : function (newValue) {
        _data.name = newValue;
    };
}

var p1 = new Person({
  name: "Paul",
  setName: function (newName) {
      this.name = newName;
  }
});
var p2 = new Person({ name: "Bob" });
p1.setName("Paul (updated)");
p2.setName("Bob (updated)");

console.log(p1.name());
console.log(p2.name());

Instead of using a primitive to store the name we use an object, whose reference we can pass as this using bind. Originally, when you wrote this._name = newValue;, this meant something quite different from what you thought, namely the object you pass the intantiate the new Person.

demo

Upvotes: 2

tomb
tomb

Reputation: 1827

You are setting this._name in your p1 Person, whereas you are accessing the 'internal' variable _name in your Person() function, instead of this._name. This can easily be fixed by adding this. to parts in your Person() class.

function Person(data) {
    this._name = data.name;

    this.name = function () {
        return this._name;
    };
    this.setName = data.setName || function (newValue) {
        this._name = newValue;
    };
}

Your second example, with the wrapping, is not working due to some errors you appear to have made in the setName() function. Firstly, you are not passing anything to the data.setName() function, meaning that it is receiving undefined for the first parameter. Next up, you are setting value to the returned value, whereas you should instead be setting _name to the returned value. Try this instead:

this.setName = data.setName ? function (newValue) {
    return (function (value) {
        _name = data.setName();
    }(newValue));
} : function (newValue) { _name = newValue; }

Upvotes: 0

Related Questions