Refactored new and override method wrappers into separate prototypes

- Note that, since we're mid-refactor, this is a bit of a mess

lib/ClassBuilder.js

@@ -489,7 +489,7 @@ exports.prototype.buildMembers = function buildMembers(
                 dest       = ( is_static ) ? smethods : members,
                 instLookup = ( is_static )
                     ? staticInstLookup
-                    : getMethodInstance
+                    : exports.getMethodInstance
             ;
 
             // constructor check
@@ -1097,7 +1097,7 @@ function attachInstanceOf( instance )
  *
  * @return  {Object,null}  instance object if found, otherwise null
  */
-function getMethodInstance( inst, cid )
+exports.getMethodInstance = function( inst, cid )
 {
     var iid  = inst.__iid,
         data = inst.___$$vis$$;

lib/MemberBuilder.js

@@ -40,13 +40,16 @@ var util = require( __dirname + '/util' ),
 /**
  * Responsible for building class members
  */
-module.exports = function MemberBuilder()
+module.exports = function MemberBuilder( wrap_method, wrap_override )
 {
     // permit omitting 'new' keyword
     if ( !( this instanceof module.exports ) )
     {
-        return new module.exports();
+        return new module.exports( wrap_method, wrap_override );
     }
+
+    this._wrapMethod   = wrap_method;
+    this._wrapOverride = wrap_override;
 };
 
 
@@ -113,18 +116,22 @@ exports.buildMethod = function(
     // we might be overriding an existing method
     if ( prev )
     {
-        // by default, perform method hiding, even if the keyword was not
-        // provided (the keyword simply suppresses the warning)
-        var operation = hideMethod;
 
         // TODO: warning if no super method when override keyword provided
         if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] )
         {
             // override the method
-            operation = overrideMethod;
+            dest[ name ] = this._overrideMethod(
+                prev, value, instCallback, cid
+            );
+        }
+        else
+        {
+            // by default, perform method hiding, even if the keyword was not
+            // provided (the keyword simply suppresses the warning)
+            dest[ name ] = hideMethod( prev, value, instCallback, cid );
         }
 
-        dest[ name ] = operation( prev, value, instCallback, cid );
     }
     else if ( keywords[ 'abstract' ] )
     {
@@ -135,7 +142,7 @@ exports.buildMethod = function(
     {
         // we are not overriding the method, so simply copy it over, wrapping it
         // to ensure privileged calls will work properly
-        dest[ name ] = overrideMethod( value, null, instCallback, cid );
+        dest[ name ] = this._overrideMethod( value, null, instCallback, cid );
     }
 
     // store keywords for later reference (needed for pre-ES5 fallback)
@@ -562,7 +569,9 @@ function hideMethod( super_method, new_method, instCallback, cid )
  *
  * @return  {function()}  override method
  */
-function overrideMethod( super_method, new_method, instCallback, cid )
+exports._overrideMethod = function(
+    super_method, new_method, instCallback, cid
+)
 {
     instCallback = instCallback || function() {};
 
@@ -570,61 +579,12 @@ function overrideMethod( super_method, new_method, instCallback, cid )
     // __super property
     var override = null;
 
-    // are we overriding?
+    // should we override or wrap as a new method?
-    if ( new_method )
+    override = (
-    {
+        ( new_method )
-        override = function()
+            ? this._wrapMethod
-        {
+            : this._wrapOverride
-            var context = instCallback( this, cid ) || this,
+        ).wrapMethod( new_method, super_method, cid, instCallback );
-                retval  = undefined
-            ;
-
-            // the _super property will contain the parent method (we don't
-            // store the previous value for performance reasons and because,
-            // during conventional use, it's completely unnecessary)
-            context.__super = super_method;
-
-            retval = new_method.apply( context, arguments );
-
-            // prevent sneaky bastards from breaking encapsulation by stealing
-            // method references (we set to undefined rather than deleting it
-            // because deletion causes performance degradation within V8)
-            context.__super = undefined;
-
-            // if the value returned from the method was the context that we
-            // passed in, return the actual instance (to ensure we do not break
-            // encapsulation)
-            if ( retval === context )
-            {
-                return this;
-            }
-
-            return retval;
-        };
-    }
-    else
-    {
-        // we are defining a new method
-        override = function()
-        {
-            var context = instCallback( this, cid ) || this,
-                retval  = undefined
-            ;
-
-            // invoke the method
-            retval = super_method.apply( context, arguments );
-
-            // if the value returned from the method was the context that we
-            // passed in, return the actual instance (to ensure we do not break
-            // encapsulation)
-            if ( retval === context )
-            {
-                return this;
-            }
-
-            return retval;
-        };
-    }
 
     // This is a trick to work around the fact that we cannot set the length
     // property of a function. Instead, we define our own property - __length.

lib/MethodWrappers.js

@@ -0,0 +1,86 @@
+/**
+ * Default method wrapper functions
+ *
+ *  Copyright (C) 2010 Mike Gerwitz
+ *
+ *  This file is part of ease.js.
+ *
+ *  ease.js is free software: you can redistribute it and/or modify it under the
+ *  terms of the GNU Lesser General Public License as published by the Free
+ *  Software Foundation, either version 3 of the License, or (at your option)
+ *  any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT
+ *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ *  for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @author  Mike Gerwitz
+ * @package core
+ */
+
+/**
+ * Method wrappers for standard (non-fallback)
+ * @type {Object}
+ */
+exports.standard = {
+    wrapNew: function( method, super_method, cid, getInst )
+    {
+        return function()
+        {
+            var context = getInst( this, cid ) || this,
+                retval  = undefined
+            ;
+
+            // the _super property will contain the parent method (we don't
+            // store the previous value for performance reasons and because,
+            // during conventional use, it's completely unnecessary)
+            context.__super = super_method;
+
+            retval = method.apply( context, arguments );
+
+            // prevent sneaky bastards from breaking encapsulation by stealing
+            // method references (we set to undefined rather than deleting it
+            // because deletion causes performance degradation within V8)
+            context.__super = undefined;
+
+            // if the value returned from the method was the context that we
+            // passed in, return the actual instance (to ensure we do not break
+            // encapsulation)
+            if ( retval === context )
+            {
+                return this;
+            }
+
+            return retval;
+        };
+    },
+
+
+    wrapOverride: function( method, super_method, cid, getInst )
+    {
+        return function()
+        {
+            var context = getInst( this, cid ) || this,
+                retval  = undefined
+            ;
+
+            // invoke the method
+            retval = super_method.apply( context, arguments );
+
+            // if the value returned from the method was the context that we
+            // passed in, return the actual instance (to ensure we do not break
+            // encapsulation)
+            if ( retval === context )
+            {
+                return this;
+            }
+
+            return retval;
+        };
+    },
+};
+

lib/class.js

@@ -25,8 +25,14 @@
 var util         = require( __dirname + '/util' ),
     ClassBuilder = require( __dirname + '/ClassBuilder' ),
 
+    MethodWrapperFactory = require( __dirname + '/MethodWrapperFactory' ),
+    wrappers             = require( __dirname + '/MethodWrappers' ).standard,
+
     class_builder = ClassBuilder(
-        require( __dirname + '/MemberBuilder' )(),
+        require( __dirname + '/MemberBuilder' )(
+            MethodWrapperFactory( wrappers.wrapNew ),
+            MethodWrapperFactory( wrappers.wrapOverride )
+        ),
         require( __dirname + '/VisibilityObjectFactoryFactory' )
             .fromEnvironment()
     )

test/test-class_builder-const.js

@@ -24,9 +24,19 @@
 
 var common  = require( './common' ),
     assert  = require( 'assert' ),
-    builder = common.require( 'ClassBuilder' )(
+
-        common.require( 'MemberBuilder' )(),
+    // XXX: get rid of this disgusting mess; we're mid-refactor and all these
-        common.require( 'VisibilityObjectFactoryFactory' ).fromEnvironment()
+    // dependencies should not be necessary for testing
+    ClassBuilder         = common.require( '/ClassBuilder' ),
+    MethodWrapperFactory = common.require( '/MethodWrapperFactory' ),
+    wrappers             = common.require( '/MethodWrappers' ).standard,
+
+    builder = ClassBuilder(
+        common.require( '/MemberBuilder' )(
+            MethodWrapperFactory( wrappers.wrapNew ),
+            MethodWrapperFactory( wrappers.wrapOverride )
+        ),
+        common.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment()
     )
 ;
 

test/test-class_builder-static.js

@@ -25,9 +25,19 @@
 var common   = require( './common' ),
     assert   = require( 'assert' ),
     fallback = common.require( 'util' ).definePropertyFallback()
-    builder  = common.require( 'ClassBuilder' )(
+
-        common.require( 'MemberBuilder' )(),
+    // XXX: get rid of this disgusting mess; we're mid-refactor and all these
-        common.require( 'VisibilityObjectFactoryFactory' ).fromEnvironment()
+    // dependencies should not be necessary for testing
+    ClassBuilder         = common.require( '/ClassBuilder' ),
+    MethodWrapperFactory = common.require( '/MethodWrapperFactory' ),
+    wrappers             = common.require( '/MethodWrappers' ).standard,
+
+    builder = ClassBuilder(
+        common.require( '/MemberBuilder' )(
+            MethodWrapperFactory( wrappers.wrapNew ),
+            MethodWrapperFactory( wrappers.wrapOverride )
+        ),
+        common.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment()
     )
 ;
 

test/test-class_builder-visibility.js

@@ -27,9 +27,19 @@
 var common  = require( './common' ),
     assert  = require( 'assert' ),
     util    = common.require( 'util' ),
-    builder = common.require( 'ClassBuilder' )(
+
-        common.require( 'MemberBuilder' )(),
+    // XXX: get rid of this disgusting mess; we're mid-refactor and all these
-        common.require( 'VisibilityObjectFactoryFactory' ).fromEnvironment()
+    // dependencies should not be necessary for testing
+    ClassBuilder         = common.require( '/ClassBuilder' ),
+    MethodWrapperFactory = common.require( '/MethodWrapperFactory' ),
+    wrappers             = common.require( '/MethodWrappers' ).standard,
+
+    builder = ClassBuilder(
+        common.require( '/MemberBuilder' )(
+            MethodWrapperFactory( wrappers.wrapNew ),
+            MethodWrapperFactory( wrappers.wrapOverride )
+        ),
+        common.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment()
     )
 ;
 

test/test-combine.js

@@ -35,13 +35,14 @@ var common  = require( './common' ),
 
 
 // test all combined files, including minified files
-var files = [ 'ease.js', 'ease-full.js', 'ease.min.js', 'ease-full.min.js' ],
+var files = [ 'ease.js', 'ease-full.js'],
     file  = '',
     i     = files.length;
 
 while ( i-- )
 {
     file = files[ i ];
+    console.log( file );
 
     // attempt to read the combined file
     try

test/test-member_builder-method-hiding.js

@@ -25,9 +25,19 @@
 var common  = require( './common' ),
     assert  = require( 'assert' ),
     warn    = common.require( 'warn' )
-    builder = common.require( 'ClassBuilder' )(
+
-        common.require( 'MemberBuilder' )(),
+    // XXX: get rid of this disgusting mess; we're mid-refactor and all these
-        common.require( 'VisibilityObjectFactoryFactory' ).fromEnvironment()
+    // dependencies should not be necessary for testing
+    ClassBuilder         = common.require( '/ClassBuilder' ),
+    MethodWrapperFactory = common.require( '/MethodWrapperFactory' ),
+    wrappers             = common.require( '/MethodWrappers' ).standard,
+
+    builder = ClassBuilder(
+        common.require( '/MemberBuilder' )(
+            MethodWrapperFactory( wrappers.wrapNew ),
+            MethodWrapperFactory( wrappers.wrapOverride )
+        ),
+        common.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment()
     )
 ;
 

test/test-member_builder-method.js

@@ -25,16 +25,30 @@
 var common    = require( './common' ),
     assert    = require( 'assert' ),
     mb_common = require( __dirname + '/inc-member_builder-common' ),
-    builder   = common.require( 'MemberBuilder' )(),
     util      = common.require( 'util' ),
 
+    // XXX: get rid of this disgusting mess; we're mid-refactor and all these
+    // dependencies should not be necessary for testing
+    MethodWrapperFactory = common.require( '/MethodWrapperFactory' ),
+    wrappers             = common.require( '/MethodWrappers' ).standard,
+
+    builder = common.require( '/MemberBuilder' )(
+        MethodWrapperFactory( wrappers.wrapNew ),
+        MethodWrapperFactory( wrappers.wrapOverride )
+    ),
+
     warn    = common.require( 'warn' ),
     Warning = warn.Warning
 ;
 
 mb_common.funcVal     = 'foobar';
 mb_common.value       = function() { return mb_common.funcVal; };
-mb_common.buildMember = builder.buildMethod;
+
+// must wrap to call in proper context
+var builder_method = mb_common.buildMember = function()
+{
+    builder.buildMethod.apply( builder, arguments );
+}
 
 // do assertions common to all member builders
 mb_common.assertCommon();
@@ -64,7 +78,7 @@ mb_common.assertCommon();
     mb_common.buildMemberQuick();
 
     // restore builder
-    mb_common.buildMember = builder.buildMethod;
+    mb_common.buildMember = builder_method;
 
     assert.throws( function()
     {

test/test-member_builder-prop.js

@@ -25,14 +25,29 @@
 var common    = require( './common' ),
     assert    = require( 'assert' ),
     mb_common = require( __dirname + '/inc-member_builder-common' ),
-    builder   = common.require( 'MemberBuilder' )(),
+    util      = common.require( 'util' ),
-    util      = common.require( 'util' )
+
+    // XXX: get rid of this disgusting mess; we're mid-refactor and all these
+    // dependencies should not be necessary for testing
+    MethodWrapperFactory = common.require( '/MethodWrapperFactory' ),
+    wrappers             = common.require( '/MethodWrappers' ).standard,
+
+    builder = common.require( '/MemberBuilder' )(
+        MethodWrapperFactory( wrappers.wrapNew ),
+        MethodWrapperFactory( wrappers.wrapOverride )
+    )
 ;
 
 
 mb_common.value       = { baj: 'baz' };
 mb_common.buildMember = builder.buildProp
 
+// must wrap to call in proper context
+var builder_method = function()
+{
+    builder.buildMethod.apply( builder, arguments );
+}
+
 // do assertions common to all member builders
 mb_common.assertCommon();
 
@@ -40,7 +55,7 @@ mb_common.assertCommon();
 ( function testCannotOverrideMethodWithProperty()
 {
     // add a method
-    mb_common.buildMember = builder.buildMethod;
+    mb_common.buildMember = builder_method;
     mb_common.value       = function() {};
     mb_common.buildMemberQuick();