Maybe a bug in model.get(), model.set() - maybe intention

[Paratron]

Hey,
I am not sure if this is happening by design or a bug.

I want to retrieve a value from a model, modify it and then set it back to the model, for example like so:

var members = mymodel.get('members');
members.push('somebody');
mymodel.set('members', members);

The problem is: since the value is returned by reference, my members.push() directly affects the content of mymodel.attributes.members.
When I call the mymodel.set(), it won't trigger a change event, since it assumes that the model has already been changed.

I would assume that .get() returns a clone of the model value to avoid unintended changes of the model. If this is the desired behavior, I would suggest to add a little notice in the docs of the model.get() function.

[Paratron]

Update:
This also happens when you call the getJSON() method of the model.

var dta = mymodel.getJSON();
dta.members.push('test');

After this code, the mymodel.attributes.members object is compromised.

[moudy]

@Paratron by getJSON do you mean toJSON? in that case a clone of the attributes is returned but it's not a deep clone so the array is copied by reference.

re your first point about model.get() returning a reference and not a clone... thats only an issue when the value is not a primitive value. i've solved this issue by just cloning it myself var members = _.clone(mymodel.get('members')) but it would be nice to not have to keep track of when that's necessary.

not sure what the solution is to avoid this confusion, maybe model.get() should return a clone of the value if it's not primitive?

[braddunbar]

Hi @Paratron! This is definitely by design. Model#get is a fairly hot code path and thus shouldn't do any cloning. Model#toJSON, on the other hand, is used for persisting the model to the server. Besides having lesser performance requirements, it is generally assumed that data may be altered in Model#toJSON before being sent to the server. Such alterations are usually not done to data returned from Model#get, so no cloning is necessary.

Edit: I'm assuming you meant Model#toJSON, not Model#getJSON. If not, please let me know.

[Paratron]

@moudy
Yes, of course, I meant toJSON(). sorry.

I am aware of the performance loss it will cause to clone every value returned by get but this way one does always have to be careful about what he's doing with the retrieved values...

I think for the beginning, I am going to extend Backbone.Model with a method gets() (short for get safe), which returns a cloned value.

[tgriesser]

@Paratron - sounds like a good option - another option as I mentioned in #2315, would be to replace the get/toJSON on the Model's prototype, if you would prefer using deep cloned attributes across your application:

Backbone.Model.prototype.toJSON = function () {
  return JSON.parse(JSON.stringify(this.attributes));
};

Backbone.Model.prototype.get = function (key) {
  var attrs = JSON.parse(JSON.stringify(this.attributes));
  return attrs[key];
};