Refactored abstract method logic out of util.propCopy() into class extend()

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;
 

lib/util.js

@@ -199,21 +199,14 @@ exports.propParse = function( data, options )
  *
  * @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}  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 },
+            var pre = dest[ name ];
-                pre  = dest[ name ];
 
             // check for override
             if ( pre !== undefined )
             {
                 if ( pre instanceof Function )
                 {
-                    dest[ name ] = method_override(
+                    // use provided method override action, or fall back to
-                        pre,
+                    // generic override
-                        func,
+                    var override_func = use_or( actions.methodOverride,
-                        name,
+                        function( name, pre, func )
-                        abstract_methods,
+                        {
-                        data
+                            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 );
+    exports.propParse( props, parse_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 );
-    }
 }
 
 
@@ -362,15 +344,16 @@ exports.isAbstractMethod = function( func )
  * @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  {Array.<string>=}  abstract_methods  list of abstract methods
- * @param  {Object}    data              object in which to store result data
  *
  * @return  {Function}  overridden method
  */
-function method_override(
+exports.overrideMethod = function(
-    super_method, new_method, name, abstract_methods, data
+    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;
         }
     }
 
@@ -425,7 +407,7 @@ function method_override(
 
         return retval;
     }
-}
+};
 
 
 /**

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() {},
@@ -49,9 +49,20 @@ var each   = false,
     prop     = false,
     method   = false,
     getter   = false,
-    setter = 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"
+);
+