From 5f739e604be73a98604faa9178d24ca4b22398e9 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 12:56:14 -0500 Subject: [PATCH 1/7] Class name is included in definition errors when available --- lib/class.js | 7 +++--- test/test-class-name.js | 50 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/lib/class.js b/lib/class.js index 59fd64f..edb9bf5 100644 --- a/lib/class.js +++ b/lib/class.js @@ -253,8 +253,8 @@ function createNamedClass( name, def ) if ( arguments.length > 2 ) { throw Error( - "Expecting two arguments for named Class definition; " + - arguments.length + " given." + "Expecting two arguments for definition of named Class '" + name + + "'; " + arguments.length + " given." ); } @@ -268,7 +268,8 @@ function createNamedClass( name, def ) else if ( typeof def !== 'object' ) { throw TypeError( - "Unexpected value for named class definition; object expected" + "Unexpected value for definition of named Class '" + name + + "'; object expected" ); } diff --git a/test/test-class-name.js b/test/test-class-name.js index 4d40508..3125e4d 100644 --- a/test/test-class-name.js +++ b/test/test-class-name.js @@ -45,15 +45,45 @@ var common = require( './common' ), "Class defined with name is returned as a valid class" ); }, Error, "Class accepts name" ); +} )(); - // the second argument must be an object - assert.throws( function() + +/** + * The class definition must be an object, which is equivalent to the class + * body + */ +( function testNamedClassDefinitionRequiresThatDefinitionBeAnObject() +{ + var name = 'Foo'; + + try { - Class( 'Foo', 'Bar' ); - }, TypeError, "Second argument to named class must be the definition" ); + Class( name, 'Bar' ); + + // if all goes well, we'll never get to this point + assert.fail( "Second argument to named class must be the definition" ); + } + catch ( e ) + { + assert.notEqual( + e.toString().match( name ), + null, + "Class definition argument count error string contains class name" + ); + } +} )(); + + +/** + * Extraneous arguments likely indicate a misunderstanding of the API + */ +( function testNamedClassDefinitionIsStrictOnArgumentCount() +{ + var name = 'Foo', + args = [ name, {}, 'extra' ] + ; // we should be permitted only two arguments - var args = [ 'Foo', {}, 'extra' ]; try { Class.apply( null, args ); @@ -66,8 +96,16 @@ var common = require( './common' ), } catch ( e ) { + var errstr = e.toString(); + assert.notEqual( - e.toString().match( args.length + ' given' ), + errstr.match( name ), + null, + "Named class error should provide name of class" + ); + + assert.notEqual( + errstr.match( args.length + ' given' ), null, "Named class error should provide number of given arguments" ); From da8be9affa205f1baba9a3bb031b6029b4d6a839 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 12:57:21 -0500 Subject: [PATCH 2/7] Interface definition errors now contain class name when available --- lib/interface.js | 7 ++--- test/test-interface-name.js | 53 ++++++++++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/lib/interface.js b/lib/interface.js index 6271930..30900eb 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -148,8 +148,8 @@ function createNamedInterface( name, def ) if ( arguments.length > 2 ) { throw Error( - "Expecting two arguments for named Interface definition; " + - arguments.length + " given." + "Expecting two arguments for definition of named Interface '" + + name + "'; " + arguments.length + " given." ); } @@ -157,7 +157,8 @@ function createNamedInterface( name, def ) if ( typeof def !== 'object' ) { throw TypeError( - "Unexpected value for named class definition; object expected" + "Unexpected value for definition of named Interface '" + + name + "'; object expected" ); } diff --git a/test/test-interface-name.js b/test/test-interface-name.js index 804678b..dc9612b 100644 --- a/test/test-interface-name.js +++ b/test/test-interface-name.js @@ -44,15 +44,48 @@ var common = require( './common' ), "Interface defined with name is returned as a valid interface" ); }, Error, "Interface accepts name" ); +} )(); - // the second argument must be an object - assert.throws( function() + +/** + * The interface definition, which equates to the body of the interface, must be + * an object + */ +( function testNamedInterfaceDefinitionRequiresThatDefinitionBeAnObject() +{ + var name = 'Foo'; + + try { - Interface( 'Foo', 'Bar' ); - }, TypeError, "Second argument to named interface must be the definition" ); + Interface( name, 'Bar' ); + + // if all goes well, we'll never get to this point + assert.fail( + "Second argument to named interface must be the definition" + ); + } + catch ( e ) + { + assert.notEqual( + e.toString().match( name ), + null, + "Interface definition argument count error string contains " + + "interface name" + ); + } +} )(); + + +/** + * Extraneous arguments likely indicate a misunderstanding of the API + */ +( function testNamedInterfaceDefinitionIsStrictOnArgumentCount() +{ + var name = 'Foo', + args = [ name, {}, 'extra' ] + ; // we should be permitted only two arguments - var args = [ 'Foo', {}, 'extra' ]; try { Interface.apply( null, args ); @@ -65,8 +98,16 @@ var common = require( './common' ), } catch ( e ) { + var errstr = e.toString(); + assert.notEqual( - e.toString().match( args.length + ' given' ), + errstr.match( name ), + null, + "Named interface error should provide interface name" + ); + + assert.notEqual( + errstr.match( args.length + ' given' ), null, "Named interface error should provide number of given arguments" ); From 2967cc7a9ad73a2fc284fb0e11a0fc7bce2ffddf Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 13:13:53 -0500 Subject: [PATCH 3/7] Class name is now provided in all errors where name is available, within class module --- lib/class.js | 10 +++++++-- test/test-class-name.js | 50 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/lib/class.js b/lib/class.js index edb9bf5..85fa979 100644 --- a/lib/class.js +++ b/lib/class.js @@ -425,7 +425,10 @@ var extend = ( function( extending ) // disallow use of our internal __initProps() method if ( name === '__initProps' ) { - throw new Error( "__initProps is a reserved method" ); + throw new Error( + ( ( cname ) ? cname + '::' : '' ) + + "__initProps is a reserved method" + ); } }, @@ -585,7 +588,10 @@ var extend = ( function( extending ) { if ( !extending ) { - throw Error( "Abstract classes cannot be instantiated" ); + throw Error( + "Abstract class " + ( cname || '(anonymous)' ) + + " cannot be instantiated" + ); } }; diff --git a/test/test-class-name.js b/test/test-class-name.js index 3125e4d..19fa1b9 100644 --- a/test/test-class-name.js +++ b/test/test-class-name.js @@ -234,3 +234,53 @@ var common = require( './common' ), ); } )(); + +/** + * The class name should be provided in the error thrown when attempting to + * instantiate an abstract class, if it's available + */ +( function testClassNameIsGivenWhenTryingToInstantiateAbstractClass() +{ + var name = 'Foo'; + + try + { + Class( name, { 'abstract foo': [] } )(); + + // we're not here to test to make sure it is thrown, but if it's not, + // then there's likely a problem + assert.fail( + "Was expecting instantiation error. There's a bug somewhere!" + ); + } + catch ( e ) + { + assert.notEqual( + e.toString().match( name ), + null, + "Abstract class instantiation error should contain class name" + ); + } + + // if no name is provided, then (anonymous) should be indicated + try + { + Class( { 'abstract foo': [] } )(); + + // we're not here to test to make sure it is thrown, but if it's not, + // then there's likely a problem + assert.fail( + "Was expecting instantiation error. There's a bug somewhere!" + ); + } + catch ( e ) + { + assert.notEqual( + e.toString().match( '(anonymous)' ), + null, + "Abstract class instantiation error should recognize that class " + + "is anonymous if no name was given" + ); + } +} )(); + From 6f7dabe35ecc9fd7ad280e413411782e7e934b14 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 17:27:02 -0500 Subject: [PATCH 4/7] Interface name is included in declaration errors, if available --- lib/interface.js | 16 +++++--- test/test-interface-name.js | 73 +++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/lib/interface.js b/lib/interface.js index 30900eb..47051d0 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -203,22 +203,25 @@ var extend = ( function( extending ) property: function() { throw TypeError( - "Properties are not permitted within Interface " + - "definitions (did you forget the 'abstract' keyword?)" + "Property not permitted within definition of " + + "Interface '" + iname + "' (did you forget the " + + "'abstract' keyword?)" ); }, getter: function() { throw TypeError( - "Getters are not permitted within Interface definitions" + "Getter not permitter within definition of Interface '" + + iname + "'" ); }, setter: function() { throw TypeError( - "Setters are not permitted within Interface definitions" + "Setter within definition of Interface '" + + iname + "'" ); }, @@ -227,8 +230,9 @@ var extend = ( function( extending ) if ( !is_abstract ) { throw TypeError( - "Only abstract methods are permitted within " + - "Interface definitions" + "Concrete method not permitted in declaration of " + + "Interface '" + iname + "'; please declare as " + + "abstract." ); } diff --git a/test/test-interface-name.js b/test/test-interface-name.js index dc9612b..a50f101 100644 --- a/test/test-interface-name.js +++ b/test/test-interface-name.js @@ -145,3 +145,76 @@ var common = require( './common' ), ); } )(); + +( function testDeclarationErrorsProvideInterfaceNameIsAvailable() +{ + var name = 'Foo', + + // functions used to cause the various errors + tries = [ + // properties + function() + { + Interface( name, { prop: 'str' } ); + }, + + // methods + function() + { + Interface( name, { method: function() {} } ); + }, + ] + ; + + // if we have getter/setter support, add those to the tests + if ( Object.defineProperty ) + { + // getter + tries.push( function() + { + var obj = {}; + Object.defineProperty( obj, 'getter', { + get: function() {}, + enumerable: true, + } ); + + Interface( name, obj ); + } ); + + // setter + tries.push( function() + { + var obj = {}; + Object.defineProperty( obj, 'setter', { + set: function() {}, + enumerable: true, + } ); + + Interface( name, obj ); + } ); + } + + // gather the error strings + var i = tries.length; + while ( i-- ) + { + try + { + // cause the error + tries[ i ](); + + // we shouldn't get to this point... + assert.fail( "Expected error. Something's wrong." ); + } + catch ( e ) + { + // ensure the error string contains the interface name + assert.notEqual( + e.toString().match( name ), + null, + "Error contains interface name when available (" + i + ")" + ); + } + } +} )(); + From b321610cc72007b3990a86e7b1fe8b67ef03fee8 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 21:46:44 -0500 Subject: [PATCH 5/7] Interface name included in instantiation error, if available --- lib/interface.js | 11 ++++++++--- test/test-interface-name.js | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/interface.js b/lib/interface.js index 47051d0..d42df51 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -197,7 +197,7 @@ var extend = ( function( extending ) // sanity check inheritCheck( prototype ); - var new_interface = createInterface(); + var new_interface = createInterface( iname ); util.propParse( props, { property: function() @@ -261,9 +261,11 @@ var extend = ( function( extending ) /** * Creates a new interface constructor function * + * @param {string=} iname interface name + * * @return {function()} */ - function createInterface() + function createInterface( iname ) { return function() { @@ -273,7 +275,10 @@ var extend = ( function( extending ) { // only called if someone tries to create a new instance of an // interface - throw Error( "Interfaces cannot be instantiated" ); + throw Error( + "Interface" + ( ( iname ) ? ( iname + ' ' ) : '' ) + + " cannot be instantiated" + ); } }; } diff --git a/test/test-interface-name.js b/test/test-interface-name.js index a50f101..dad582c 100644 --- a/test/test-interface-name.js +++ b/test/test-interface-name.js @@ -218,3 +218,26 @@ var common = require( './common' ), } } )(); + +( function testInterfaceNameIsIncludedInInstantiationError() +{ + var name = 'Foo'; + + try + { + // this should throw an exception (cannot instantiate interfaces) + Interface( name )(); + + // we should never get here + assert.fail( "Exception expected. There's a bug somewhere." ); + } + catch ( e ) + { + assert.notEqual( + e.toString().match( name ), + null, + "Interface name is included in instantiation error message" + ); + } +} )(); + From dfa2966d9955dd669ad7fba81fed357c8294aaa8 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 22:55:15 -0500 Subject: [PATCH 6/7] Removed class naming from TODO and updated README to include mention --- README.md | 9 +++++++++ TODO | 2 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 72ae34e..d51aa15 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,15 @@ class. The constructor is provided as the `__construct()` method (influenced by }, }); +The above creates an anonymous class and stores it in the variable ``Foo``. You +have the option of naming class in order to provide more useful error messages +and toString() output: + + var Foo = Class( 'Foo', + { + // ... + }); + ### Extending Classes Classes may inherit from one-another. If the supertype was created using `Class.extend()`, a convenience `extend()` method has been added to it. Classes diff --git a/TODO b/TODO index d56accc..c703ff4 100644 --- a/TODO +++ b/TODO @@ -12,8 +12,6 @@ Misc functions, will not impact function logic. - Should be able to run source file without preprocessing, so C-style macros 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 From d6cca750937a82ba58fd20d343d289324ef60bb9 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 5 Mar 2011 23:11:13 -0500 Subject: [PATCH 7/7] Can now override toString() method of a class --- lib/class.js | 35 +++++++++++++++++++++-------------- test/test-class-extend.js | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/lib/class.js b/lib/class.js index 85fa979..ec9e3cc 100644 --- a/lib/class.js +++ b/lib/class.js @@ -489,7 +489,7 @@ var extend = ( function( extending ) prototype.parent = base.prototype; // set up the new class - var new_class = createCtor( cname, abstract_methods ); + var new_class = createCtor( cname, abstract_methods, members ); attachPropInit( prototype, properties ); @@ -523,10 +523,11 @@ var extend = ( function( extending ) * * @param {string} cname class name (may be empty) * @param {Array.} abstract_methods list of abstract methods + * @param {Object} members class members * * @return {Function} constructor */ - function createCtor( cname, abstract_methods ) + function createCtor( cname, abstract_methods, members ) { // concrete class if ( abstract_methods.__length === 0 ) @@ -559,18 +560,24 @@ var extend = ( function( extending ) // constructor to ensure they are not overridden) attachInstanceOf( this ); - // provide a more intuitive string representation of the class - // instance - this.toString = ( cname ) - ? function() - { - return '[object #<' + cname + '>]'; - } - : function() - { - return '[object #]'; - } - ; + // Provide a more intuitive string representation of the class + // instance. If a toString() method was already supplied for us, + // use that one instead. + if ( !( Object.prototype.hasOwnProperty.call( + members[ 'public' ], 'toString' + ) ) ) + { + this.toString = ( cname ) + ? function() + { + return '[object #<' + cname + '>]'; + } + : function() + { + return '[object #]'; + } + ; + } }; // provide a more intuitive string representation diff --git a/test/test-class-extend.js b/test/test-class-extend.js index a82f61f..6a78ac0 100644 --- a/test/test-class-extend.js +++ b/test/test-class-extend.js @@ -281,3 +281,29 @@ for ( var i = 0; i < class_count; i++ ) } } )(); + +/** + * We provide a useful default toString() method, but one may wish to override + * it + */ +( function testCanOverrideToStringMethod() +{ + var str = 'foomookittypoo', + result = '' + ; + + result = Class( 'Foo', + { + toString: function() + { + return str; + } + })().toString(); + + assert.equal( + result, + str, + "Can override default toString() method of class" + ); +} )(); +