From b158e542d5f1e114c1dfe490b15a6031753d70d7 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 4 Mar 2011 16:36:15 -0500 Subject: [PATCH 01/10] Declaring named class will throw error for extreaneous arguments --- lib/class.js | 12 +++++++++++- test/test-class-name.js | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/class.js b/lib/class.js index 7ef56f6..332f6cd 100644 --- a/lib/class.js +++ b/lib/class.js @@ -67,7 +67,7 @@ module.exports = function() if ( arguments.length > 1 ) { throw Error( - "Expecting one argument for Class definition; " + + "Expecting one argument for anonymous Class definition; " + arguments.length + " given." ); } @@ -79,6 +79,16 @@ module.exports = function() name = arguments[ 0 ]; def = arguments[ 1 ]; + // if too many arguments were provided, it's likely that they're + // expecting some result that they're not going to get + if ( arguments.length > 2 ) + { + throw Error( + "Expecting two arguments for named Class definition; " + + arguments.length + " given." + ); + } + // add the name to the definition def.__name = name; diff --git a/test/test-class-name.js b/test/test-class-name.js index aa05a5b..360c943 100644 --- a/test/test-class-name.js +++ b/test/test-class-name.js @@ -49,6 +49,27 @@ var common = require( './common' ), { Class( 'Foo', 'Bar' ); }, TypeError, "Second argument to named class must be the definition" ); + + // we should be permitted only two arguments + var args = [ 'Foo', {}, 'extra' ]; + try + { + Class.apply( null, args ); + + // we should not get to this line (an exception should be thrown due to + // too many arguments) + assert.fail( + "Should accept only two arguments when creating named class" + ); + } + catch ( e ) + { + assert.notEqual( + e.toString().match( args.length + ' given' ), + null, + "Named class error should provide number of given arguments" + ); + } } )(); From e3075b0479541fc98252bc618f5bb76e78f323e2 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 4 Mar 2011 19:49:15 -0500 Subject: [PATCH 02/10] Refactored class module invocation method into separate functions - The class module is getting too big. Something will have to be done soon. --- lib/class.js | 103 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 41 deletions(-) diff --git a/lib/class.js b/lib/class.js index 332f6cd..3d9e0ef 100644 --- a/lib/class.js +++ b/lib/class.js @@ -50,56 +50,20 @@ var class_meta = {}; */ module.exports = function() { - var def = {}, - name = '', - type = typeof arguments[ 0 ] + var type = typeof arguments[ 0 ], + result = null ; switch ( type ) { // anonymous class case 'object': - def = arguments[ 0 ]; - - // ensure we have the proper number of arguments (if they passed in - // too many, it may signify that they don't know what they're doing, - // and likely they're not getting the result they're looking for) - if ( arguments.length > 1 ) - { - throw Error( - "Expecting one argument for anonymous Class definition; " + - arguments.length + " given." - ); - } - + result = createAnonymousClass.apply( null, arguments ); break; // named class case 'string': - name = arguments[ 0 ]; - def = arguments[ 1 ]; - - // if too many arguments were provided, it's likely that they're - // expecting some result that they're not going to get - if ( arguments.length > 2 ) - { - throw Error( - "Expecting two arguments for named Class definition; " + - arguments.length + " given." - ); - } - - // add the name to the definition - def.__name = name; - - // the definition must be an object - if ( typeof def !== 'object' ) - { - throw TypeError( - "Unexpected value for named class definition" - ); - } - + result = createNamedClass.apply( null, arguments ); break; default: @@ -109,7 +73,7 @@ module.exports = function() ); } - return extend( def ); + return result; }; @@ -251,6 +215,63 @@ module.exports.isA = module.exports.isInstanceOf; function Class() {}; +/** + * Creates a new anonymous Class from the given class definition + * + * @param {Object} def class definition + * + * @return {Class} new anonymous class + */ +function createAnonymousClass( def ) +{ + // ensure we have the proper number of arguments (if they passed in + // too many, it may signify that they don't know what they're doing, + // and likely they're not getting the result they're looking for) + if ( arguments.length > 1 ) + { + throw Error( + "Expecting one argument for anonymous Class definition; " + + arguments.length + " given." + ); + } + + return extend( def ); +} + + +/** + * Creates a new named Class from the given class definition + * + * @param {string} name class name + * @param {Object} def class definition + * + * @return {Class} new named class + */ +function createNamedClass( name, def ) +{ + // if too many arguments were provided, it's likely that they're + // expecting some result that they're not going to get + if ( arguments.length > 2 ) + { + throw Error( + "Expecting two arguments for named Class definition; " + + arguments.length + " given." + ); + } + + // add the name to the definition + def.__name = name; + + // the definition must be an object + if ( typeof def !== 'object' ) + { + throw TypeError( + "Unexpected value for named class definition" + ); + } + + return extend( def ); +} /** From ea5797ec4b79dc7de22a451418fafdf1aecdfe9d Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 4 Mar 2011 19:50:02 -0500 Subject: [PATCH 03/10] Added class module refactoring (due to side) to TODO --- TODO | 1 + 1 file changed, 1 insertion(+) diff --git a/TODO b/TODO index fbc7f27..d56accc 100644 --- a/TODO +++ b/TODO @@ -14,6 +14,7 @@ Misc cannot be used (# is not a valid token) - Class/Interface naming - Will be especially helpful for error messages + - Class module is becoming too large; refactor Property Keywords - Restrictions; throw exceptions when unknown keywords are used From 649356ef23b36a1ca274d6054b0e0e04ae23f4e7 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 4 Mar 2011 23:35:28 -0500 Subject: [PATCH 04/10] Refactored interface module invocation into separate functions for named and anonymous --- lib/interface.js | 69 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/lib/interface.js b/lib/interface.js index 1ae2ce3..2ca594d 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -42,38 +42,20 @@ var util = require( './util' ), */ module.exports = function() { - var def = {}, - name = '', - type = typeof arguments[ 0 ] + var type = typeof arguments[ 0 ] + result = null ; switch ( type ) { // anonymous interface case 'object': - def = arguments[ 0 ]; - - // ensure we have the proper number of arguments (if they passed in - // too many, it may signify that they don't know what they're doing, - // and likely they're not getting the result they're looking for) - if ( arguments.length > 1 ) - { - throw Error( - "Expecting one argument for Interface definition; " + - arguments.length + " given." - ); - } - + result = createAnonymousInterface.apply( null, arguments ); break; // named class case 'string': - name = arguments[ 0 ]; - def = arguments[ 1 ]; - - // add the name to the definition - def.__name = name; - + result = createNamedInterface.apply( null, arguments ); break; default: @@ -84,7 +66,7 @@ module.exports = function() ); } - return extend( def ); + return result; }; @@ -127,6 +109,47 @@ module.exports.isInterface = function( obj ) function Interface() {} +/** + * Creates a new anonymous Interface from the given interface definition + * + * @param {Object} def interface definition + * + * @return {Interface} new anonymous interface + */ +function createAnonymousInterface( def ) +{ + // ensure we have the proper number of arguments (if they passed in + // too many, it may signify that they don't know what they're doing, + // and likely they're not getting the result they're looking for) + if ( arguments.length > 1 ) + { + throw Error( + "Expecting one argument for Interface definition; " + + arguments.length + " given." + ); + } + + return extend( def ); +} + + +/** + * Creates a new named interface from the given interface definition + * + * @param {string} name interface name + * @param {Object} def interface definition + * + * @return {Interface} new named interface + */ +function createNamedInterface( name, def ) +{ + // add the name to the definition + def.__name = name; + + return extend( def ); +} + + var extend = ( function( extending ) { return function extend() From 0ccdf07145d8a31b3c0da8bf1c15b838e8929d7e Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 4 Mar 2011 23:43:30 -0500 Subject: [PATCH 05/10] Implemented strict argument check for interface creation --- lib/interface.js | 18 ++++++++++++++++++ test/test-interface-name.js | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/lib/interface.js b/lib/interface.js index 2ca594d..6271930 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -143,6 +143,24 @@ function createAnonymousInterface( def ) */ function createNamedInterface( name, def ) { + // if too many arguments were provided, it's likely that they're + // expecting some result that they're not going to get + if ( arguments.length > 2 ) + { + throw Error( + "Expecting two arguments for named Interface definition; " + + arguments.length + " given." + ); + } + + // the definition must be an object + if ( typeof def !== 'object' ) + { + throw TypeError( + "Unexpected value for named class definition; object expected" + ); + } + // add the name to the definition def.__name = name; diff --git a/test/test-interface-name.js b/test/test-interface-name.js index cfe5689..804678b 100644 --- a/test/test-interface-name.js +++ b/test/test-interface-name.js @@ -50,6 +50,27 @@ var common = require( './common' ), { Interface( 'Foo', 'Bar' ); }, TypeError, "Second argument to named interface must be the definition" ); + + // we should be permitted only two arguments + var args = [ 'Foo', {}, 'extra' ]; + try + { + Interface.apply( null, args ); + + // we should not get to this line (an exception should be thrown due to + // too many arguments) + assert.fail( + "Should accept only two arguments when creating named interface" + ); + } + catch ( e ) + { + assert.notEqual( + e.toString().match( args.length + ' given' ), + null, + "Named interface error should provide number of given arguments" + ); + } } )(); From bedc3c95af8ab2ba1c0ace1323bf35f78d20568f Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 4 Mar 2011 23:44:19 -0500 Subject: [PATCH 06/10] Moved named class argument check error to a more sensible location and amended error message --- lib/class.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/class.js b/lib/class.js index 3d9e0ef..b89c4d8 100644 --- a/lib/class.js +++ b/lib/class.js @@ -259,17 +259,17 @@ function createNamedClass( name, def ) ); } - // add the name to the definition - def.__name = name; - // the definition must be an object if ( typeof def !== 'object' ) { throw TypeError( - "Unexpected value for named class definition" + "Unexpected value for named class definition; object expected" ); } + // add the name to the definition + def.__name = name; + return extend( def ); } From ace9f4c1ea6f15537a3cd8453134b8b8b0177ef6 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 00:33:47 -0500 Subject: [PATCH 07/10] Began implementing class name staging (implement() does not yet work with it) --- lib/class.js | 41 ++++++++++++++++++++++++++++++++++++++++- test/test-class-name.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/lib/class.js b/lib/class.js index b89c4d8..44c4411 100644 --- a/lib/class.js +++ b/lib/class.js @@ -259,8 +259,14 @@ function createNamedClass( name, def ) ); } + // if no definition was given, return a staging object, to apply the name to + // the class once it is actually created + if ( def === undefined ) + { + return createStaging( name ); + } // the definition must be an object - if ( typeof def !== 'object' ) + else if ( typeof def !== 'object' ) { throw TypeError( "Unexpected value for named class definition; object expected" @@ -274,6 +280,39 @@ function createNamedClass( name, def ) } +/** + * Creates a staging object to stage a class name + * + * The class name will be applied to the class generated by operations performed + * on the staging object. This allows applying names to classes that need to be + * extended or need to implement interfaces. + * + * @param {string} cname desired class name + * + * @return {Object} object staging the given class name + */ +function createStaging( cname ) +{ + return { + extend: function() + { + var args = Array.prototype.slice.apply( arguments ); + + // extend() takes a maximum of two arguments. If only one + // argument is provided, then it is to be the class definition. + // Otherwise, the first argument is the supertype and the second + // argument is the class definition. Either way you look at it, + // the class definition is always the final argument. + // + // We want to add the name to the definition. + args[ args.length - 1 ].__name = cname; + + return extend.apply( null, args ); + }, + }; +} + + /** * Creates extend function * diff --git a/test/test-class-name.js b/test/test-class-name.js index 360c943..6fbf3fa 100644 --- a/test/test-class-name.js +++ b/test/test-class-name.js @@ -149,3 +149,32 @@ var common = require( './common' ), ); } )(); + +/** + * In order to accommodate syntax such as extending classes, ease.js supports + * staging class names. This will return an object that operates exactly like + * the normal Class module, but will result in a named class once the class is + * created. + */ +( function testCanCreateNamedClassUsingStagingMethod() +{ + var name = 'Foo', + named = Class( name ).extend( {} ) + ; + + // ensure what was returned is a valid class + assert.equal( + Class.isClass( named ), + true, + "Named class generated via staging method is considered to be a " + + "valid class" + ); + + // was the name set? + assert.equal( + named.toString(), + '[object Class <' + name + '>]', + "Name is set on named clas via staging method" + ); +} )(); + From 0f9454b79b14e46873758642a3f6fd27e954eada Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 02:59:21 -0500 Subject: [PATCH 08/10] interface() no longer returns a usable class - Number of reasons. First and foremost, it doesn't make sense to return a usable class with no body/definition - Secondly, to make the following commit(s) possible and consistent --- lib/class.js | 52 +++++++++++++++++++++++++++------- test/test-class-implement.js | 55 +++++++++++++++++++++++++++++------- 2 files changed, 87 insertions(+), 20 deletions(-) diff --git a/lib/class.js b/lib/class.js index 44c4411..33b8ccf 100644 --- a/lib/class.js +++ b/lib/class.js @@ -100,12 +100,11 @@ module.exports.extend = function( base ) */ module.exports.implement = function() { - var args = Array.prototype.slice.call( arguments ); - - // apply to an empty (new) object - args.unshift( module.exports.extend() ); - - return implement.apply( this, args ); + // implement on empty base + return createImplement( + module.exports.extend(), + Array.prototype.slice.call( arguments ) + ); }; @@ -313,6 +312,37 @@ function createStaging( cname ) } +/** + * Creates an intermediate object to permit implementing interfaces + * + * This object defers processing until extend() is called. This intermediate + * object ensures that a usable class is not generated until after extend() is + * called, as it does not make sense to create a class without any + * body/definition. + * + * @param {Object} base base class to implement atop of + * @param {Array} ifaces interfaces to implement + * + * @return {Object} intermediate implementation object + */ +function createImplement( base, ifaces ) +{ + ifaces.unshift( base ); + + // Defer processing until after extend(). This also ensures that implement() + // returns nothing usable. + return { + extend: function( def ) + { + return extend.apply( null, [ + implement.apply( this, ifaces ), + def + ] ); + }, + }; +} + + /** * Creates extend function * @@ -714,6 +744,8 @@ function attachExtend( func ) /** * Attaches implement method to the given function (class) * + * Please see the implement() export of this module for more information. + * * @param {function()} func function (class) to attach method to * * @return {undefined} @@ -722,10 +754,10 @@ function attachImplement( func ) { util.defineSecureProp( func, 'implement', function() { - var args = Array.prototype.slice.call( arguments ); - args.unshift( func ); - - return implement.apply( this, args ); + return createImplement( + func, + Array.prototype.slice.call( arguments ) + ); }); } diff --git a/test/test-class-implement.js b/test/test-class-implement.js index cfb973a..ff46d61 100644 --- a/test/test-class-implement.js +++ b/test/test-class-implement.js @@ -64,19 +64,41 @@ var Type = Interface.extend( { { assert.doesNotThrow( function() { - Foo = Class.implement( Type, Type2 ); + Class.implement( Type, Type2 ); }, Error, "Class can implement interfaces" ); +} )(); - assert.ok( - ( Class.isClass( Foo ) ), - "Class returned from implementing interfaces on an empty base is a " + - "valid class" + +/** + * Initially, the implement() method returned an abstract class. However, it + * doesn't make sense to create a class without any actual definition (and + * there's other implementation considerations that caused this route to be + * taken). One wouldn't do "class Foo implements Type", and not provide any + * body. + * + * Therefore, implement() should return nothing useful until extend() is called + * on it. + */ +( function testResultOfImplementIsNotUsableAsAClass() +{ + var result = Class.implement( Type ); + + assert.equal( + ( Class.isClass( result ) ), + false, + "Result of implement operation on class is not usable as a Class" ); } )(); +/** + * As a consequence of the above, we must extend with an empty definition + * (base) in order to get our abstract class. + */ ( function testAbstractMethodsCopiedIntoNewClassUsingEmptyBase() { + Foo = Class.implement( Type, Type2 ).extend( {} ); + assert.ok( ( ( Foo.prototype.foo instanceof Function ) && ( Foo.prototype.foo2 instanceof Function ) @@ -90,19 +112,32 @@ var Type = Interface.extend( { { assert.doesNotThrow( function() { - PlainFoo2 = PlainFoo.implement( Type, Type2 ); + PlainFoo.implement( Type, Type2 ); }, Error, "Classes can implement interfaces" ); +} )(); - assert.ok( - ( Class.isClass( PlainFoo2 ) ), - "Class returned from implementing interfaces on an existing base is a " + - "valid class" + +/** + * Ensure the same system mentioned above also applies to the extend() method on + * existing classes + */ +( function testImplementingInterfaceAtopExistingClassIsNotUsableByDefault() +{ + var result = PlainFoo.implement( Type ); + + assert.equal( + ( Class.isClass( result ) ), + false, + "Result of implementing interfaces on an existing base is not " + + "usable as a Class" ); } )(); ( function testAbstractMethodsCopiedIntoNewClassUsingExistingBase() { + PlainFoo2 = PlainFoo.implement( Type, Type2 ).extend( {} ); + assert.ok( ( ( PlainFoo2.prototype.foo instanceof Function ) && ( PlainFoo2.prototype.foo2 instanceof Function ) From 874922876406be1baef7dcde6672d1f6094f68b7 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 03:03:50 -0500 Subject: [PATCH 09/10] interface() calls now push/pop rather than shift/unshift (performance) --- lib/class.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/class.js b/lib/class.js index 33b8ccf..effbdb5 100644 --- a/lib/class.js +++ b/lib/class.js @@ -327,7 +327,7 @@ function createStaging( cname ) */ function createImplement( base, ifaces ) { - ifaces.unshift( base ); + ifaces.push( base ); // Defer processing until after extend(). This also ensures that implement() // returns nothing usable. @@ -602,7 +602,7 @@ var implement = function() { var args = Array.prototype.slice.call( arguments ), dest = {}, - base = args.shift(), + base = args.pop(), len = args.length, arg = null, From 2c2701f4abb14df955eff16c0b9868c111c07584 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 03:22:45 -0500 Subject: [PATCH 10/10] Implemented staging-style named class definition when implementing interfaces --- lib/class.js | 24 +++++++++++++++++++++--- test/test-class-name.js | 24 +++++++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/lib/class.js b/lib/class.js index effbdb5..59fd64f 100644 --- a/lib/class.js +++ b/lib/class.js @@ -308,6 +308,17 @@ function createStaging( cname ) return extend.apply( null, args ); }, + + implement: function() + { + // implement on empty base, providing the class name to be used once + // extended + return createImplement( + extend( {} ), + Array.prototype.slice.call( arguments ), + cname + ); + }, }; } @@ -320,12 +331,13 @@ function createStaging( cname ) * called, as it does not make sense to create a class without any * body/definition. * - * @param {Object} base base class to implement atop of - * @param {Array} ifaces interfaces to implement + * @param {Object} base base class to implement atop of + * @param {Array} ifaces interfaces to implement + * @param {string=} cname optional class name once extended * * @return {Object} intermediate implementation object */ -function createImplement( base, ifaces ) +function createImplement( base, ifaces, cname ) { ifaces.push( base ); @@ -334,6 +346,12 @@ function createImplement( base, ifaces ) return { extend: function( def ) { + // if a name was provided, use it + if ( cname ) + { + def.__name = cname; + } + return extend.apply( null, [ implement.apply( this, ifaces ), def diff --git a/test/test-class-name.js b/test/test-class-name.js index 6fbf3fa..4d40508 100644 --- a/test/test-class-name.js +++ b/test/test-class-name.js @@ -24,7 +24,9 @@ var common = require( './common' ), assert = require( 'assert' ), - Class = common.require( 'class' ) + + Class = common.require( 'class' ), + Interface = common.require( 'interface' ) ; @@ -158,8 +160,9 @@ var common = require( './common' ), */ ( function testCanCreateNamedClassUsingStagingMethod() { - var name = 'Foo', - named = Class( name ).extend( {} ) + var name = 'Foo', + named = Class( name ).extend( {} ) + namedi = Class( name ).implement( Interface( {} ) ).extend( {} ) ; // ensure what was returned is a valid class @@ -176,5 +179,20 @@ var common = require( './common' ), '[object Class <' + name + '>]', "Name is set on named clas via staging method" ); + + + // we should also be able to implement interfaces + assert.equal( + Class.isClass( namedi ), + true, + "Named class generated via staging method, implementing an " + + "interface, is considered to be a valid class" + ); + + assert.equal( + namedi.toString(), + '[object Class <' + name + '>]', + "Name is set on named class via staging method when implementing" + ); } )();