Objects in class definitions are shared across instances

[mquan]

This example http://jsfiddle.net/t9uRW/3/ demonstrates some unexpected behavior when creating a new object for an ArrayProxy subclass, namely its content isn't set to default value. Seems like it's reusing the content of the previous instance. Same behavior observed with ArrayController.

[wagenet]

This isn't strictly a bug, though it may be an area we can improve. When creating an instance, we just pass along the same value defined in the class. If it's a literal, there's no problem, but if it's an object, it ends up being shared across instances. This sharing could be a problem depending on your case. One solution is to write an init method along the lines of:

init: function() {
  this._super();
  this.set('content', []);
}

It may be possible to setup things such that objects are cloned, but it would be difficult to tell the proper way to clone the object since that will vary by type.

[ralfschimmel]

This is truly unexpected behavior which we ran into on multiple occasions in the last two weeks of coding Ember.
Why would an instance ever want to share a property with it's fellow instances?

The content of a property of an instance is unique by definition.
http://en.wikipedia.org/wiki/Instance_(computer_science)

[wagenet]

@ralfschimmel The instances are unique, but they both point to the same object that is not unique.

[wagenet]

Consider the following case:

var parent = Ember.Object.create();

MyApp.Child = Ember.Object.extend({
  parent: parent
});

var child1 = MyApp.Child.create(),
    child2 = MyApp.Child.create();

Should child1.get('parent') === child2.get('parent')? I sort of feel like it should, but if not, how do you clone the parent? (There's Ember.copy, but what if it's not an Ember object?)

[krisselden]

This is just an often confusing aspect of Javascript, objects inherit from objects, there are no classes. This prototype is a shared instance. What you pass to ember extend becomes a prototype for what you create.

http://javascript.crockford.com/prototypal.html

http://en.wikipedia.org/wiki/Prototype-based_programming

Create does call init on the resulting instance so you can setup instance properties.

[ralfschimmel]

@wagenet I agree with your use case. Though the term .create() implies different. When using .extend one expects a Class and not an Object (at least, I do), when using .create one expects an Object of the Class, not a copy/paste of the Class as defined with .extend (which is actually an Object).

It's more a problem in the definition then an actual problem with Ember, though I think we can agree that Ember could handle this Javascript lack of classical inheritance in a better way.

ps. we are a Java company which makes the confusion as described by @kselden even more true

[vincentheet]

For example the Javascript MVC framework fixes the in my Java developer eyes 'strange' Javascript prototypal inheritance see: http://javascriptmvc.com/docs.html#!jQuery.Class
And also see John Resign's example implementation: http://ejohn.org/blog/simple-javascript-inheritance/

The real question is should Ember.js provide the more classical way of inheritance (as an option). Personally I think this would be a great feature to have in Ember.

[wagenet]

@ralfschimmel, @vincentheet How would you propose my above example be handled? Unless you can suggest a way to handle this consistently, I don't see that there's much we can do.

[ralfschimmel]

Well it seems to me that we should not be passing initialized objects to a Class. I would define the Class with parent: null (using extend) and then set the parent when initializing the object using create, that feels more OO.

This however totally depends on the bigger ideas behind Ember, do you want it to mimic OO in that way or not?

[wagenet]

@ralfschimmel, I agree that my example doesn't really provide a model of good practice. However, the problem you run into, is that [] is also an instantiated object of the type Array. It's essentially the same issue.

[vincentheet]

@wagenet Do you mean it is an issue because it is an array? Maybe this would be helpful then: http://my.opera.com/GreyWyvern/blog/show.dml/1725165

Or maybe using jQuery: http://stackoverflow.com/questions/122102/what-is-the-most-efficient-way-to-clone-a-javascript-object

[wagenet]

@vincentheet, @ralfschimmel said "it seems to me that we should not be passing initialized objects to a Class". My comment was that [] is an initialized object. So it seems fair to say that you shouldn't pass [] into a class definition either.

And I do know how to use jQuery extend to clone objects, but sometimes there is more to the object than just data that needs to be handled. Also, I think that the intended behavior here is still ambiguous.

I think we should maintain the current behavior and consider adding a new API that tells what properties should be "defaults" (or cloned on instantiation).

[ralfschimmel]

Extending the API was also our conclusion when discussing it yesterday morning with @vincentheet. Something like a .instance() to accompany the .extend() and .create() sounds like a possible solution for our problem.

[wagenet]

Can you say more about instance()? It's definitely not what I had in mind and I have some doubts about it, but it might help to hear more detail on what you were thinking.

[wycats]

We've discussed having copyableProperties (like concatenatedProperties) before. The main issue with that is that while copying Arrays is easy and fast, copying objects is not, and you could easily get into a situation where poorly considered app code ends up putting deep cloning a deeply nested object on the hot path.

This could happen, for instance, if you added a copyableProperty to a view you used often that was a deeply nested object.

Further, people would probably expect putting arbitrary Ember objects in a copyable property to work, and that's even more complicated (and potentially slow).

[wagenet]

Given the complexity of doing this right coupled with the fact that there is a workaround and no one has a solid proposal on how to do this, I'm inclined to close this. @wycats?

[wagenet]

This, of course, is not to say I wouldn't welcome suggestions or pull requests to implement this feature.

[ralfschimmel]

Ok, the concerns seem valid :-)

We implemented our framework using the init method to initialize instances. The resulting code is not very neat and the use case for creating an instance, like for example in Java, remains.

[wagenet]

@ralfschimmel Having an initialize method that does some setup doesn't seem that crazy to me. We might be able to make some things a little simpler, but honestly, it doesn't seem that complex to me as it is.

[wagenet]

Taking lack of feedback from @wycats as a go-ahead to close.

[Restuta]

@wycats I think it can be solved for arrays. bit not arbitrary objects. I feel like "shared arrays" is much more common behaviour and can lead to very hard to debug issues pretty easily.

[Restuta]

I would also mention this in Ember Docs under Object Model section.