From 600e389b407161859a057f42260d84425d2854b7 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 19 Dec 2010 00:11:39 -0500 Subject: [PATCH] Refactored abstract method logic out of util.propCopy() into class extend() --- lib/class.js | 23 ++++++++++- lib/util.js | 78 ++++++++++++++----------------------- test/test-util-prop-copy.js | 53 ++++++++++++++++++++----- 3 files changed, 95 insertions(+), 59 deletions(-) diff --git a/lib/class.js b/lib/class.js index 70fab6e..c3046ad 100644 --- a/lib/class.js +++ b/lib/class.js @@ -93,7 +93,7 @@ var extend = ( function( extending ) }; var properties = {}; - util.propCopy( props, prototype, result_data, { + util.propCopy( props, prototype, { each: function( name, value ) { // disallow use of our internal __initProps() method @@ -110,8 +110,29 @@ var extend = ( function( extending ) properties[ name ] = value; this.performDefault( name, value ); }, + + method: function( name, func, is_abstract ) + { + var pre = prototype[ name ]; + + if ( ( pre === undefined ) && is_abstract ) + { + result_data.abstractMethods.push( name ); + } + + this.performDefault( name, func, is_abstract ); + }, + + methodOverride: function( name, pre, func ) + { + return util.overrideMethod( + pre, func, name, result_data.abstractMethods + ); + }, } ); + result_data.abstractMethods = util.arrayShrink( result_data.abstractMethods ); + // reference to the parent prototype (for more experienced users) prototype.parent = base.prototype; diff --git a/lib/util.js b/lib/util.js index 11c657b..c526092 100644 --- a/lib/util.js +++ b/lib/util.js @@ -197,23 +197,16 @@ exports.propParse = function( data, options ) * The result data will be populated with information from the copy that may be * useful to the creation of the class (e.g. list of the abstract methods). * - * @param {Object} props properties to copy - * @param {Object} dest destination object - * @param {Object} result_data object to store data regarding the copy in + * @param {Object} props properties to copy + * @param {Object} dest destination object + * @param {Object} actions parser actions (see propParse()) * * @return undefined */ -exports.propCopy = function( props, dest, result_data, actions ) +exports.propCopy = function( props, dest, actions ) { - result_data = result_data || {}; actions = actions || {}; - // initialize result_data - var abstract_methods = - result_data.abstractMethods = result_data.abstractMethods || []; - - var abstract_regen = false; - var use_or = function( use, or ) { if ( use instanceof Function ) @@ -228,7 +221,7 @@ exports.propCopy = function( props, dest, result_data, actions ) }; // substitute default functionality if needed - actions = { + var parse_actions = { each: use_or( actions.each, function( name, value ) { // methods can only be overridden with methods @@ -257,20 +250,26 @@ exports.propCopy = function( props, dest, result_data, actions ) method: use_or( actions.method, function( name, func, is_abstract ) { - var data = { abstractModified: false }, - pre = dest[ name ]; + var pre = dest[ name ]; // check for override if ( pre !== undefined ) { if ( pre instanceof Function ) { - dest[ name ] = method_override( - pre, - func, - name, - abstract_methods, - data + // use provided method override action, or fall back to + // generic override + var override_func = use_or( actions.methodOverride, + function( name, pre, func ) + { + return exports.overrideMethod( pre, func, name ); + } + ); + + // use call(), passing self in as context, to ensure 'this' + // will reference the function itself + dest[ name ] = override_func.call( + override_func, name, pre, func ); } else @@ -279,33 +278,16 @@ exports.propCopy = function( props, dest, result_data, actions ) "Cannot override property '" + name + "' with method" ); } - - if ( data.abstractModified ) - { - abstract_regen = true; - } } else { // simply copy over the method dest[ name ] = func; - - if ( is_abstract ) - { - abstract_methods.push( name ); - } } } ), }; - exports.propParse( props, actions ); - - // should we regenerate the array of abstract methods? (this must be done - // because the length of the array remains the same after deleting elements) - if ( abstract_regen ) - { - result_data.abstractMethods = exports.arrayShrink( abstract_methods ); - } + exports.propParse( props, parse_actions ); } @@ -359,18 +341,19 @@ exports.isAbstractMethod = function( func ) * * The given method must be a function or an exception will be thrown. * - * @param {Function} super_method method to override - * @param {Function} new_method method to override with - * @param {string} name method name - * @param {Array} abstract_methods list of abstract methods - * @param {Object} data object in which to store result data + * @param {Function} super_method method to override + * @param {Function} new_method method to override with + * @param {string} name method name + * @param {Array.=} abstract_methods list of abstract methods * * @return {Function} overridden method */ -function method_override( - super_method, new_method, name, abstract_methods, data +exports.overrideMethod = function( + super_method, new_method, name, abstract_methods ) { + abstract_methods = abstract_methods || []; + // it's much faster to lookup a hash than it is to iterate through an entire // array each time we need to find an existing abstract method var abstract_map = {}; @@ -406,7 +389,6 @@ function method_override( if ( is_abstract === false ) { delete abstract_methods[ abstract_map[ name ] ]; - data.abstractModified = true; } } @@ -420,12 +402,12 @@ function method_override( // assign _super temporarily for the method invocation so // that the method can call the parent method this.__super = super_method; - var retval = new_method.apply( this, arguments ); + var retval = new_method.apply( this, arguments ); this.__super = tmp; return retval; } -} +}; /** diff --git a/test/test-util-prop-copy.js b/test/test-util-prop-copy.js index aa16470..1b3ce20 100644 --- a/test/test-util-prop-copy.js +++ b/test/test-util-prop-copy.js @@ -31,7 +31,7 @@ var props = { one: 1, two: 2, - method: function() {}, + method: function two() {}, get val() {}, set val() {}, @@ -45,13 +45,24 @@ assert.ok( "All properties should be copied to the destination object" ); -var each = false, - prop = false, - method = false, - getter = false, - setter = false; +var each = false, + prop = false, + method = false, + getter = false, + setter = false, + override = false, -propCopy( props, dest, null, { + override_data = []; + +var test_val = 'foobar', + dest2_orig_method = function() { return test_val; }; + +var dest2 = { + // will cause methodOverride action to be invoked + method: dest2_orig_method, +}; + +propCopy( props, dest2, { each: function foo() { each = this.performDefault; @@ -62,9 +73,10 @@ propCopy( props, dest, null, { prop = this.performDefault; }, - method: function() + method: function( name, func ) { - method = this.performDefault; + // perform default action to ensure methodOverride() is called + ( method = this.performDefault )( name, func ); }, getter: function() @@ -76,9 +88,15 @@ propCopy( props, dest, null, { { setter = this.performDefault; }, + + methodOverride: function( name, pre, func ) + { + override = this.performDefault; + override_data = [ name, pre, func ]; + }, } ); -[ each, prop, method, getter, setter ].forEach( function( item, i ) +[ each, prop, method, getter, setter, override ].forEach( function( item, i ) { assert.notEqual( item, @@ -93,3 +111,18 @@ propCopy( props, dest, null, { ); }); +assert.ok( + ( override_data[ 0 ] === 'method' ), + "methodOverride action is passed correct method name" +); + +assert.ok( + ( override_data[ 1 ]() === test_val ), + "methodOverride action is passed correct original function" +); + +assert.ok( + ( override_data[ 2 ] === props.method ), + "methodOverride action is passed correct override function" +); +