patternjavascriptMinor
Modification of javascript inheritance framework
Viewed 0 times
javascriptframeworkinheritancemodification
Problem
I've modified John Resig simple javascript inheritance framework, by adding set function with setter functionality. It seems that it works. Is it written correctly? Can you see some undesired behavior?
Framework:
Test code:
```
var Person = Class.extend({
a: 15,
b: 30,
init: function(){
//...
},
on_change:{
b: function(oldKey, set){this[oldKey] = set*2}
}
});
var p
Framework:
(function(){
var initializing = false, fnTest = /xyz/.test(function(){xyz;}) ? /\b_super\b/ : /.*/;
this.Class = function(){};
Class.extend = function(prop) {
var _super = this.prototype;
initializing = true;
var prototype = new this();
initializing = false;
for (var name in prop) {
prototype[name] = typeof prop[name] == "function" &&
typeof _super[name] == "function" && fnTest.test(prop[name]) ?
(function(name, fn){
return function() {
var tmp = this._super;
this._super = _super[name];
var ret = fn.apply(this, arguments);
// CHANGED HERE FROM this._super = tmp;
// don't like some not used properties
if(tmp)this._super = tmp;
// END OF CHANGE
return ret;
};
})(name, prop[name]) :
prop[name];
}
function Class() {
if ( !initializing && this.init )
this.init.apply(this, arguments);
}
Class.prototype = prototype;
Class.prototype.constructor = Class;
Class.extend = arguments.callee;
// CHANGED HERE
// Set method
Class.prototype.set = function(attrs) {
for (var attr in attrs) {
//if exist setter function this.on_change{key of wrap: function(key of wrap, new value)
if(this.on_change[attr])this.on_change[attr].call(this,attr,attrs[attr]);
else this[attr] = attrs[attr];
}
return this;
};
if(!Class.prototype.on_change)Class.prototype.on_change={};
// END OF CHANGE
return Class;
};
})();
Test code:
```
var Person = Class.extend({
a: 15,
b: 30,
init: function(){
//...
},
on_change:{
b: function(oldKey, set){this[oldKey] = set*2}
}
});
var p
Solution
Easy question first : Is it written correctly?
I would counter suggest:
Hard question: Can you see some undesired behavior?
Yes.
-
You are counting on the setter to do the assignment, another possible source of bugs. I would do the assignment regardless, rename
Class.prototype.set = function(attrs) {
for (var attr in attrs) {
//if exist setter function this.on_change{key of wrap: function(key of wrap, new value)
if(this.on_change[attr])this.on_change[attr].call(this,attr,attrs[attr]);
else this[attr] = attrs[attr];
}
return this;
};
if(!Class.prototype.on_change)Class.prototype.on_change={};attrsis not a good name, it does not contain a list of attributes, but a list of name/value pairs, I would suggest the Spartanofor Object or maybemap.
- Dropping the curly braces in your if statement is ok, dropping the newlines not so much
- The one line of comment is not very helpful :\
- You could use the following for initialization of
on_change:
Class.prototype.on_change = Class.prototype.on_change || {};on_changeis not lowerCamelCase ->onChangewould be more idiomatic.
I would counter suggest:
Class.prototype.set = function( map ){
for ( var attributeName in map ) {
var value = map[attributeName],
setter = this.onChange[attributeName];
if( setter )
setter.call( this, attributeName, value );
else
this[attributeName] = value;
}
return this;
};
Class.prototype.onChange = Class.prototype.onChange || {};Hard question: Can you see some undesired behavior?
Yes.
- Providing
onChangeis old skool, real events use addEventListener.
- There are now 2 ways to change a property, thru
setand thru changing properties directly. The setter functions only work forset, this will introduce bugs.
-
You are counting on the setter to do the assignment, another possible source of bugs. I would do the assignment regardless, rename
setter to listener and then execute the listener:this[attributeName] = value;
if( listener )
listener.call( this, attributeName, value );Code Snippets
Class.prototype.set = function(attrs) {
for (var attr in attrs) {
//if exist setter function this.on_change{key of wrap: function(key of wrap, new value)
if(this.on_change[attr])this.on_change[attr].call(this,attr,attrs[attr]);
else this[attr] = attrs[attr];
}
return this;
};
if(!Class.prototype.on_change)Class.prototype.on_change={};Class.prototype.set = function( map ){
for ( var attributeName in map ) {
var value = map[attributeName],
setter = this.onChange[attributeName];
if( setter )
setter.call( this, attributeName, value );
else
this[attributeName] = value;
}
return this;
};
Class.prototype.onChange = Class.prototype.onChange || {};this[attributeName] = value;
if( listener )
listener.call( this, attributeName, value );Context
StackExchange Code Review Q#6598, answer score: 3
Revisions (0)
No revisions yet.