PDA

View Full Version : [CLOSED] [4.0.2a] Class inheritance keeps references to original fields



Nick99
26 Jul 2011, 12:36 PM
Hi,

I have 2 classes, one of which inherits another. I first create an instance of the Person class and populate its fields (test, items1, wrappedItems.wrap); the contents of the fields is alerted. After that I create a Developer and alert fields values; the inherited fields MUST be empty - but they are NOT (except for the "test" fields).

The example demonstrates the behaviour. My understanding is that the second alert should output all NULLs/empty values.



Ext.define('Person', {
test: null,
items1: [],
wrappedItems: {
wrap: null
},

constructor: function(name) {
this.test = name;
this.items1.push(name);
this.wrappedItems.wrap = name;

alert("Person: " + this.items1.toString() + ", " + this.test + ", " + this.wrappedItems.wrap);
return this;
}
});
Ext.define('Developer', {
extend: 'Person',

constructor: function() {
alert("Developer: " + this.items1.toString() + ", " + this.test + ", " + this.wrappedItems.wrap);
return this;
}
});

var person = new Person("test123");
var dev = new Developer();

ykey
26 Jul 2011, 6:47 PM
This is not a problem with inheritance. It appears to be a problem with object properties. But not sure it is a problem or just a misunderstanding.

For example:




var fn = function() {};
var fn2 = function() {};

Ext.define('Person', {
name: 'Default',
data: {},
fn: undefined,

constructor: function(name, fn) {
if (name) {
this.name = name;
this.data.name = name;
}

if (fn) {
this.fn = fn;
}
}
});

Ext.onReady(function() {
var person = Ext.create('Person', "Name", fn);
var person2 = Ext.create('Person', "Name2", fn2);

console.log(person.name); // Expected
console.log(person.data.name); // Failure, expecting: Name Actual: Name2
console.log(person.fn === fn); // Expected

console.log(person2.name); // Expected
console.log(person2.data.name); // Expected
console.log(person2.fn === fn2); // Expected

console.log(person.fn !== person2.fn); // Expected
});

evant
26 Jul 2011, 7:18 PM
@ykey is correct

When you specify a property in a class definition it gets shared across all instances. Doesn't matter for strings and numbers, but you need to be careful with arrays/object literals.

ykey
26 Jul 2011, 7:28 PM
So the correct way to do this would be to create the array/object literals in the constructor or elsewhere in the instance.

Nick99
26 Jul 2011, 8:25 PM
@ykey
Thank you, you've shown the exactly same behaviour in your code.

@evant
I actualy thought those "class properties" were private to each class instance. Shouldn't be they (in normal OOP)? Instead they are more like "protected static".

So,

1. Is this the standard behaviour which won't change in next versions? It woud be nice to have the clarification somewhere @ http://docs.sencha.com/ext-js/4-0/#/guide/class_system

2. While the suggestion to put all the init code into constructor is ok as a workaround, this is somewhat clumsy. I have a class extended from Ext.window.Window, which defines "items" property. It is pretty big and I'd need to put it into constructor now, otherwise I can't create multiple instances of the class ;)

Thank you in advance.

ykey
26 Jul 2011, 8:42 PM
I am pretty sure I have narrowed this down and it really just comes down to standard JavaScript and prototypal inheritance.



function Person(name) {
if (name) {
this.name = name;
this.data.name = name;
}
}

Person.prototype.name = 'Default';
Person.prototype.data = {};

var person = new Person();
var person2 = new Person('Name2');

console.log(person.name); // Default
console.log(person.data.name); // Name2
console.log(person2.name); // Name2
console.log(person2.data.name); // Name2



Ext.Class constructor is calling Ext.Base.implement with provided arguments which just adds them to the prototype.



Ext.define('Person', {
constructor: function(name) {
if (name) {
this.name = name;
this.data.name = name;
}
}
});

Person.implement({
name: 'Default',
data: {
name: 'Default'
}
});

var person = new Person();
var person2 = new Person('Name2');

console.log(person.name); // Default
console.log(person.data.name); // Name2
console.log(person2.name); // Name2
console.log(person2.data.name); // Name2

Nick99
26 Jul 2011, 9:05 PM
Right, it is pure js.

The Ext.clone() call fixes this "pecularity". It seems it can be done automatically for Object and Array types somewhere in the Class framework.



function Person(name) {
this.obj = Ext.clone(this.obj);
//this.date = Ext.clone(this.date);

this.name = name;
this.obj.name = name;
this.fn = function() {return name;};
if (name == "second") {
this.date.setTime(0); // 1970
}
}

Person.prototype.name = 'Default';
Person.prototype.obj = {};
Person.prototype.fn = function(){};
Person.prototype.date = new Date(); // Current

var person1 = new Person("first");
var person2 = new Person("second");
console.log(person1.name); // first
console.log(person1.obj.name); // Expected: first, real: second
console.log(person1.fn()); // first
console.log(person1.date); // Expected: current, real: 1970


Update: the Date is vulnerable too.

Nick99
27 Jul 2011, 12:21 AM
I've crafted a patch to change the default prototype inheritance behaviour. Also, I wonder why I didn't encounter this problem in ExtJS 1/2/3...

Next steps:
Wait for answers/clarification from Sencha team and then close the thread.



(function() {
var overridenClass = Ext.Class;

Ext.Class = Class = function(newClass, classData, onClassCreated) {
// ext-all-debug-w-comments.js @ 5392
if (typeof newClass !== 'function') {
onClassCreated = classData;
classData = newClass;
newClass = function() {
for (key in this) {
if (key == "self" || key == "superclass" || key == "$class" ||
key == "mixins" || key == "requires" || key == "statics") continue;

if (this[key] != null && typeof(this[key]) == "object") {
this[key] = Ext.clone(this[key]);
}
}

return this.constructor.apply(this, arguments);
};
}

overridenClass.call(this, newClass, classData, onClassCreated);
}
}());

mberrie
27 Jul 2011, 2:45 AM
As I understand it this is related to prototype inheritance and cannot easily be changed.

Cloning the property upon instantiation only works for simple objects. Once you have objects that hold references to other objects, it gets pretty ugly (Do you do a deep clone or not? And how deep?) and it will seriously mess things up.

When comparing this behavior to classic OOP and compiled languages, you have to consider that for class fields the actual code will be executed upon each instantiation of the class. This is not possible in Javascript since the code 'line' that defines the class field will be evaluated upon class definition (well, actually upon creation of the config object literal) and the result of the evaluation will be passed to Ext's class manager.

So, as a developer, the tools to deal with this behavior are


understanding and awareness
Ext.apply - this will always add properties to the actual object , overriding properties inherited from the prototype chain
hasOwnProperty - allows to check whether a property is inherited or a direct member of the object
best practices


Not pretending to say absolute truths here, this is just my understanding :)

mberrie

Jacky Nguyen
7 Aug 2011, 1:50 PM
Summary:

- It's simply how JavaScript prototypal inheritance works.
- Unless the object / array members of the class are immutable, define them in the class constructor.
- Consider using the config feature.

natelapp
9 Aug 2011, 5:51 AM
This seems like a fairly significant thing to me.

I don't know that it's "simply how JavaScript prototypal inheritance work" especially since Ext is controlling it by adding everything to the prototype of the class instead of the individual instance. As it stands right now it appears to be automatically treating any object or array defined at the class level as a static. However, you've given the ability to define "statics" so I don't know why you wouldn't leverage that configuration when dealing with any property, including arrays/objects.

Is this something that Sencha would be willing to reconsider?

mberrie
9 Aug 2011, 7:48 AM
I agree that this is indeed pretty puzzling for programmers coming from a different background and applying classic OOP principles and terms to prototype-based inheritance.

However, in Javascript the only(?) way to allow a property to be inherited is to add it to the prototype.

Javascript handles resolving properties on an object (like obj.property) and traverses the prototype chain to find a match if the object itself does not define it (result: inheritance)

Any object can override the property by re-assigning a different value (this.property=..). The property will then be owned by that object alone.

It has been pointed out in this thread before, that for immutable values the behavior appears to be that of classic-OOP class fields.
The confusion starts once you do not *reassign* (override) the property value but manipulate/modify it.



How could Ext provide a solution to this?

Looking at classic OOP and compiled languages (or languages that have the construct of a 'class'), you have to consider that the code statement that defines the (default) value for a class field is re-evaluated each time an instance of the class is created.

This is not possible in Javascript and in Ext.define(). The code statement that defines the value for the property will be evaluated exactly once and the *result* will be passed to Ext.define! The class object that Ext creates from the class definition cannot really imitate the behavior described above.
One suggestion was to clone the object and assign a copy to every object that is created as instance of this class. But as I pointed out in my previous post, cloning only works for simple objects.



hth

natelapp
9 Aug 2011, 10:16 AM
That does make sense... even if I'm not happy about it... :)

I can see how simply always doing a clone wouldn't necessarily be a good idea, especially if one of those properties happened to be a reference to some other instance.

Well, I'm glad I came across this thread. And thanks for the detailed explanation. That could have made for some especially insidious bugs down the road.

Jacky Nguyen
9 Aug 2011, 10:55 AM
Compare these 2 pieces of code:


Ext.define('Foo', {
bar: {
baz: true
},

constructor: function() {
this.bar = Ext.clone(this.self.prototype.bar);
}
});



Ext.define('Foo', {
constructor: function() {
this.bar = {
baz: true
};
}
});

You will immediately find that it doesn't make sense sacrificing (huge) performance to clone to original object for every instance of Foo, instead of just creating a new one inside the class' constructor.