0

Consider a simple factory which self-updates -

function MyFactory(){

  var _data = []; // initial value

  function onLoad(response){

    // assign the incoming value to our '_data' variable
    _data = response;

  }

  // some sort of ajax to get the data
  function getData() {

     // ajax.fecth('/url', onLoad);

     // pseudo-ajax
     onLoad([4,5,6]);

  }

  // public methods / properties
  this.getData = getData;
  this.data = _data;

}

(Can you see the problem here? Worth taking a second.) I'll spell it out -

// create the object
var factory = new MyFactory();

// get the data
console.log(factory.data);  // [] ok

// get new data [1,2,3], and assign to the data property
factory.getData();

// what have we now got?
console.log(factory.data); // [] argh, expected [1,2,3]

This is not what we want at all. this.data should return the new array... but doesn't. It continues to reference the original array, even though we've internally updated our object.

There is a solution - instead of replacing the value of data, we can do this -

function onLoad(response){
    // take care to reuse the object
    _data.length = 0;
    Array.prototype.push.apply(_data, response);
}

... which is okay, but feels a bit of a hack.

My question is - what would be a better pattern of MyFactory which would ensure that we can update its data property so that it always returns the expected value.

1
  • factory.data is different from the variable data within the myFactory function. factory.data is this.data inside the myFactory function, Commented Dec 20, 2015 at 20:30

3 Answers 3

1

In addition to MinusFour's answer, you have created a bit of confusion with getData. Usually, a getter gets a property of an object that is private. It's counterpart is setter that sets the value.

Your getData actually sets the "private" value (which is a closure), then you try to read it as a property. So if you just create a new getter and change getData to a setter, you're done:

function MyFactory(){

  var _data = []; // initial value

  function onLoad(response){
    _data = response;
  }

  function getData() {
     onLoad([4,5,6]);
  }

  // setter to get data and set the value of _data
  this.setData = getData;

  // getter to return the value of _data
  this.getData = function() {
    return _data;
  };
}

// create the object
var factory = new MyFactory();

// get the data
document.write('data: ' + factory.getData());  // [] ok

// get new data [1,2,3], and assign to the data property
factory.setData();

// what have we now got?
document.write('<br>data: ' + factory.getData()); // expected [4,5,6]

You could overload the getter so that if you provide an argument, it sets the value immediately, or if no value is provided, do the AJAX thing.

Alternatively, you can define data as a getter:

function MyFactory(){
  var _data = [];
  function onLoad(response){_data = response;}
  function getData() {onLoad([4,5,6]);}

  // public methods - setter and getter
  this.getData = getData;
  
  // Create a data property as a getter
  Object.defineProperty(this, 'data', {get: function() {return _data;}});
}


var factory = new MyFactory();
document.write('Data: ' + factory.data);  // [] ok
factory.getData();

// what have we now got?
document.write('<br>Data: ' + factory.data); // expected [4,5,6]

Sign up to request clarification or add additional context in comments.

Comments

1

You probably think that by doing:

this.data = _data;

You are actually linking both variables and whatever happens to _data will happen to this.data. This is not the case, it just end up copying the object reference. Which means that both variables are pointing to the same object reference (let's say the object is "A")

this.data === A
_data === A

What your onLoad method does is create a new object (lets call it "B") and assign it to data.

this.data === A
_data === B

_data is not part of the object unlike this.data, so it's not accessible from the object. This is why it doesn't work.

There's really no reason to work this way. You obviously want the data to be public, there's no reason to keep it in the constructor scope.

function MyFactory(){

  var that = this; // object reference which private methods can use

  function onLoad(response){

    // assign the incoming value to our '_data' variable
    that.data = response;

  }

  // some sort of ajax to get the data
  function getData() {

     // ajax.fecth('/url', onLoad);

     // pseudo-ajax
     onLoad([4,5,6]);

  }

  // public methods / properties
  this.getData = getData;
  this.data = [];

}

4 Comments

There is no need for that, the methods should always be called on their base object in this case.
@RobG, the problem is that onLoad is not part of the object. I assumed it's a private function hence why it's not attached. So you either save a reference to the object or use call, apply when invoking the private function.
Ah, you changed data to be public… I prefer to use an instance name, e.g. var factory = this rather than that, which has no semantic meaning.
I feel like it's a common practice though, either that or self. The important thing is to keep the object reference somewhere accessible to the private method. An arrow function might work better here I think, that way no binding need be done.
0

Consider method naming conception. Create public getter method to access your private data on demand.

function MyFactory(){

  var _data = []; // initial value

  function onLoad(response){

    // assign the incoming value to our '_data' variable
    _data = response;

  }

  // some sort of ajax to get the data
  function loadData() {

     // ajax.fecth('/url', onLoad);

     // pseudo-ajax
     onLoad([4,5,6]);

  }

  // public methods / properties
  this.loadData = loadData;
  this.getData = function(){
      return _data;
  }

}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.