From 078555c3a2300e72c3cf969e3d67b23f1e948021 Mon Sep 17 00:00:00 2001 From: snaterlicious Date: Tue, 14 Jan 2014 10:34:55 +0100 Subject: [PATCH 1/3] Removed dependency on dataTypes --- .jshintrc | 1 - composer.json | 3 +- js/ValueFormatters.resources.php | 1 - js/ValueParsers.resources.php | 1 - .../ValueFormatters/ValueFormatterFactory.js | 56 ++++---- .../ValueFormatters/mw.ext.valueFormatters.js | 4 +- js/src/ValueParsers/ValueParserFactory.js | 59 ++++----- js/src/ValueParsers/mw.ext.valueParsers.js | 16 +-- .../ValueFormatterFactory.tests.js | 90 ++++++++----- .../ValueParsers/ValueParserFactory.tests.js | 122 +++++++++--------- 10 files changed, 180 insertions(+), 173 deletions(-) diff --git a/.jshintrc b/.jshintrc index fe0c96b..654ee30 100644 --- a/.jshintrc +++ b/.jshintrc @@ -28,7 +28,6 @@ // Globals "predef": [ - "dataTypes", // TODO: Remove dataTypes dependency "dataValues", "globeCoordinate", "jQuery", diff --git a/composer.json b/composer.json index f5c5a6c..28ece87 100644 --- a/composer.json +++ b/composer.json @@ -27,8 +27,7 @@ }, "require": { "composer/installers": ">=1.0.1", - "data-values/interfaces": "~0.1", - "data-values/data-types": "0.*,>=0.1.1" + "data-values/interfaces": "~0.1" }, "autoload": { "files" : [ diff --git a/js/ValueFormatters.resources.php b/js/ValueFormatters.resources.php index a9da133..f8553dc 100644 --- a/js/ValueFormatters.resources.php +++ b/js/ValueFormatters.resources.php @@ -40,7 +40,6 @@ 'ValueFormatterFactory.js', ), 'dependencies' => array( - 'dataTypes', 'valueFormatters', ), ), diff --git a/js/ValueParsers.resources.php b/js/ValueParsers.resources.php index 1695c70..cab834e 100644 --- a/js/ValueParsers.resources.php +++ b/js/ValueParsers.resources.php @@ -44,7 +44,6 @@ 'ValueParserFactory.js', ), 'dependencies' => array( - 'dataTypes', 'valueParsers', ), ), diff --git a/js/src/ValueFormatters/ValueFormatterFactory.js b/js/src/ValueFormatters/ValueFormatterFactory.js index 6e1103f..d8e9bb6 100644 --- a/js/src/ValueFormatters/ValueFormatterFactory.js +++ b/js/src/ValueFormatters/ValueFormatterFactory.js @@ -2,7 +2,7 @@ * @licence GNU GPL v2+ * @author H. Snater < mediawiki@snater.com > */ -( function( $, vf, dt ) { +( function( $, vf ) { 'use strict'; /** @@ -41,21 +41,22 @@ * Registers a formatter. * @since 0.1 * - * @param {dataTypes.DataType|string} purpose DataType object or DataValue type. * @param {Function} Formatter ValueFormatter constructor. + * @param {string} dataValueType + * @param {string} [dataTypeId] * * @throws {Error} if the formatter constructor is invalid. * @throws {Error} no proper purpose is provided. */ - registerFormatter: function( purpose, Formatter ) { + registerFormatter: function( Formatter, dataValueType, dataTypeId ) { if( !$.isFunction( Formatter ) ) { throw new Error( 'Invalid ValueFormatter constructor' ); } - if( purpose instanceof dt.DataType ) { - this._registerDataTypeFormatter( purpose, Formatter ); - } else if( typeof purpose === 'string' ) { - this._registerDataValueFormatter( purpose, Formatter ); + if( typeof dataTypeId === 'string' ) { + this._registerDataTypeFormatter( Formatter, dataTypeId ); + } else if( typeof dataValueType === 'string' ) { + this._registerDataValueFormatter( Formatter, dataValueType ); } else { throw new Error( 'No sufficient purpose provided what to register the formatter ' + 'for' ); @@ -66,32 +67,32 @@ * Registers a formatter for a certain data type. * @since 0.1 * - * @param {dataTypes.DataType} dataType * @param {Function} Formatter + * @param {string} dataTypeId * * @throws {Error} if a formatter for the specified dataType object is registered already. */ - _registerDataTypeFormatter: function( dataType, Formatter ) { + _registerDataTypeFormatter: function( Formatter, dataTypeId ) { assertIsValueFormatterConstructor( Formatter ); - if( this._formattersForDataTypes[dataType.getId()] ) { - throw new Error( 'Formatter for DataType "' + dataType.getId() + '" is registered ' + if( this._formattersForDataTypes[dataTypeId] ) { + throw new Error( 'Formatter for DataType "' + dataTypeId + '" is registered ' + 'already' ); } - this._formattersForDataTypes[dataType.getId()] = Formatter; + this._formattersForDataTypes[dataTypeId] = Formatter; }, /** * Registers a formatter for a certain data value type. * @since 0.1 * - * @param {string} dataValueType * @param {Function} Formatter + * @param {string} dataValueType * * @throws {Error} if a formatter for the specified DataValue type is registered already. */ - _registerDataValueFormatter: function( dataValueType, Formatter ) { + _registerDataValueFormatter: function( Formatter, dataValueType ) { assertIsValueFormatterConstructor( Formatter ); if( this._formattersForDataValueTypes[dataValueType] ) { @@ -107,32 +108,23 @@ * default formatter if no ValueFormatter is registered for that purpose. * @since 0.1 * - * @param {dataTypes.DataType|string} purpose DataType object or DataValue type. + * @param {string} dataValueType + * @param {string} [dataTypeId] * @return {Function|null} * * @throws {Error} if no proper purpose is provided to retrieve a formatter. */ - getFormatter: function( purpose ) { - var dataValueType, - dataTypeId, - formatter; - - if( purpose instanceof dataTypes.DataType ) { - dataValueType = purpose.getDataValueType(); - dataTypeId = purpose.getId(); - } else if( typeof purpose === 'string' ) { - dataValueType = purpose; - } else { - throw new Error( 'No sufficient purpose provided for choosing a formatter' ); - } + getFormatter: function( dataValueType, dataTypeId ) { + var formatter; - if( dataTypeId ) { + if( typeof dataTypeId === 'string' ) { formatter = this._formattersForDataTypes[dataTypeId]; } - if( !formatter ) { - // No formatter for specified data type or only DataValue provided. + if( !formatter && typeof dataValueType === 'string' ) { formatter = this._formattersForDataValueTypes[dataValueType]; + } else if( !formatter ) { + throw new Error( 'No sufficient purpose provided for choosing a formatter' ); } return formatter || this._DefaultFormatter; @@ -149,4 +141,4 @@ } } -}( jQuery, valueFormatters, dataTypes ) ); +}( jQuery, valueFormatters ) ); diff --git a/js/src/ValueFormatters/mw.ext.valueFormatters.js b/js/src/ValueFormatters/mw.ext.valueFormatters.js index 56851cb..7b9e029 100644 --- a/js/src/ValueFormatters/mw.ext.valueFormatters.js +++ b/js/src/ValueFormatters/mw.ext.valueFormatters.js @@ -12,8 +12,8 @@ var valueFormatterProvider = new vf.ValueFormatterFactory( vf.NullFormatter ); valueFormatterProvider.registerFormatter( - dv.StringValue.TYPE, - vf.StringFormatter + vf.StringFormatter, + dv.StringValue.TYPE ); /** diff --git a/js/src/ValueParsers/ValueParserFactory.js b/js/src/ValueParsers/ValueParserFactory.js index fd4f2de..5235e0f 100644 --- a/js/src/ValueParsers/ValueParserFactory.js +++ b/js/src/ValueParsers/ValueParserFactory.js @@ -2,7 +2,7 @@ * @licence GNU GPL v2+ * @author H. Snater < mediawiki@snater.com > */ -( function( $, vp, dt ) { +( function( $, vp ) { 'use strict'; /** @@ -41,23 +41,24 @@ * Registers a parser. * @since 0.1 * - * @param {dataTypes.DataType|string} purpose DataType object or DataValue type. * @param {Function} Parser ValueParser constructor. + * @param {string} dataValueType + * @param {string} [dataTypeId] * * @throws {Error} if the parser constructor is invalid. * @throws {Error} no proper purpose is provided. */ - registerParser: function( purpose, Parser ) { + registerParser: function( Parser, dataValueType, dataTypeId ) { if( !$.isFunction( Parser ) ) { throw new Error( 'Invalid ValueParser constructor' ); } - if( purpose instanceof dt.DataType ) { - this._registerDataTypeParser( purpose, Parser ); - } else if( typeof purpose === 'string' ) { - this._registerDataValueParser( purpose, Parser ); + if( typeof dataTypeId === 'string' ) { + this._registerDataTypeParser( Parser, dataTypeId ); + } else if( typeof dataValueType === 'string' ) { + this._registerDataValueParser( Parser, dataValueType ); } else { - throw new Error( 'No sufficient purpose provided what to register the parser for' ); + throw new Error( 'No sufficient purpose to register the parser for provided' ); } }, @@ -65,32 +66,31 @@ * Registers a parser for a certain data type. * @since 0.1 * - * @param {dataTypes.DataType} dataType * @param {Function} Parser + * @param {string} dataTypeId * * @throws {Error} if a parser for the specified dataType object is registered already. */ - _registerDataTypeParser: function( dataType, Parser ) { + _registerDataTypeParser: function( Parser, dataTypeId ) { assertIsValueParserConstructor( Parser ); - if( this._parsersForDataTypes[dataType.getId()] ) { - throw new Error( 'Parser for DataType "' + dataType.getId() + '" is registered ' - + 'already' ); + if( this._parsersForDataTypes[dataTypeId] ) { + throw new Error( 'Parser for DataType "' + dataTypeId + '" is registered already' ); } - this._parsersForDataTypes[dataType.getId()] = Parser; + this._parsersForDataTypes[dataTypeId] = Parser; }, /** * Registers a parser for a certain data value type. * @since 0.1 * - * @param {string} dataValueType * @param {Function} Parser + * @param {string} dataValueType * * @throws {Error} if a parser for the specified DataValue type is registered already. */ - _registerDataValueParser: function( dataValueType, Parser ) { + _registerDataValueParser: function( Parser, dataValueType ) { assertIsValueParserConstructor( Parser ); if( this._parsersForDataValueTypes[dataValueType] ) { @@ -106,32 +106,23 @@ * parser if no ValueParser is registered for that purpose. * @since 0.1 * - * @param {dataTypes.DataType|string} purpose DataType object or DataValue type. + * @param {string} dataValueType + * @param {string} [dataTypeId] * @return {Function|null} * * @throws {Error} if no proper purpose is provided to retrieve a parser. */ - getParser: function( purpose ) { - var dataValueType, - dataTypeId, - parser; - - if( purpose instanceof dataTypes.DataType ) { - dataValueType = purpose.getDataValueType(); - dataTypeId = purpose.getId(); - } else if( typeof purpose === 'string' ) { - dataValueType = purpose; - } else { - throw new Error( 'No sufficient purpose provided for choosing a parser' ); - } + getParser: function( dataValueType, dataTypeId ) { + var parser; - if( dataTypeId ) { + if( typeof dataTypeId === 'string' ) { parser = this._parsersForDataTypes[dataTypeId]; } - if( !parser ) { - // No parser for specified data type or only DataValue provided. + if( !parser && typeof dataValueType === 'string' ) { parser = this._parsersForDataValueTypes[dataValueType]; + } else if( !parser ) { + throw new Error( 'No sufficient purpose provided for choosing a parser' ); } return parser || this._DefaultParser; @@ -148,4 +139,4 @@ } } -}( jQuery, valueParsers, dataTypes ) ); +}( jQuery, valueParsers ) ); diff --git a/js/src/ValueParsers/mw.ext.valueParsers.js b/js/src/ValueParsers/mw.ext.valueParsers.js index 44b0a74..63df7f7 100644 --- a/js/src/ValueParsers/mw.ext.valueParsers.js +++ b/js/src/ValueParsers/mw.ext.valueParsers.js @@ -12,23 +12,23 @@ var valueParserProvider = new vp.ValueParserFactory( vp.NullParser ); valueParserProvider.registerParser( - dv.GlobeCoordinateValue.TYPE, - vp.GlobeCoordinateParser + vp.GlobeCoordinateParser, + dv.GlobeCoordinateValue.TYPE ); valueParserProvider.registerParser( - dv.QuantityValue.TYPE, - vp.QuantityParser + vp.QuantityParser, + dv.QuantityValue.TYPE ); valueParserProvider.registerParser( - dv.StringValue.TYPE, - vp.StringParser + vp.StringParser, + dv.StringValue.TYPE ); valueParserProvider.registerParser( - dv.TimeValue.TYPE, - vp.TimeParser + vp.TimeParser, + dv.TimeValue.TYPE ); /** diff --git a/js/tests/ValueFormatters/ValueFormatterFactory.tests.js b/js/tests/ValueFormatters/ValueFormatterFactory.tests.js index 366c0c5..391f38f 100644 --- a/js/tests/ValueFormatters/ValueFormatterFactory.tests.js +++ b/js/tests/ValueFormatters/ValueFormatterFactory.tests.js @@ -2,27 +2,40 @@ * @licence GNU GPL v2+ * @author H. Snater < mediawiki@snater.com > */ -( function( $, QUnit, dt, dv, vf ) { +( function( $, QUnit, dv, vf ) { 'use strict'; + var DataTypeMock = function( dataTypeId, DataValue ) { + this._dataTypeId = dataTypeId; + this._dataValueType = DataValue.TYPE; + }; + $.extend( DataTypeMock.prototype, { + getId: function() { + return this._dataTypeId; + }, + getDataValueType: function() { + return this._dataValueType; + } + } ); + /** * Returns a descriptive string to be used as id when registering a ValueFormatter in a * ValueFormatterFactory. * - * @param {dataTypes.DataType|string} purpose + * @param {DataTypeMock|Function} purpose * @return {string} */ function getTypeInfo( purpose ) { - if( purpose instanceof dt.DataType ) { + if( purpose instanceof DataTypeMock ) { return 'DataType with data value type "' + purpose.getDataValueType() + '"'; } - return 'constructor for DataValue of type "' + purpose + '"'; + return 'constructor for DataValue of type "' + purpose.TYPE + '"'; } var StringValue = dv.StringValue, UnknownValue = dv.UnknownValue, - stringType = new dt.DataType( 'somestringtype', StringValue ), - numberType = new dt.DataType( 'somenumbertype', dv.NumberValue ), + stringType = new DataTypeMock( 'somestringtype', StringValue ), + numberType = new DataTypeMock( 'somenumbertype', dv.NumberValue ), StringFormatter = vf.StringFormatter, NullFormatter = vf.NullFormatter; @@ -42,19 +55,19 @@ assert.throws( function() { - formatterFactory.registerFormatter( StringValue.TYPE, 'invalid' ); + formatterFactory.registerFormatter( 'invalid', StringValue ); }, 'Failed trying to register an invalid formatter constructor.' ); assert.throws( function() { - formatterFactory.register( 'invalid', StringFormatter ); + formatterFactory.register( StringFormatter, 'invalid' ); }, 'Failed trying to register a formatter with an invalid purpose.' ); - formatterFactory.registerFormatter( StringValue.TYPE, StringFormatter ); + formatterFactory.registerFormatter( StringFormatter, StringValue.TYPE ); assert.throws( function() { @@ -73,7 +86,7 @@ 'Returning default formatter if no formatter is registered for a specific data value.' ); - formatterFactory.registerFormatter( StringValue.TYPE, StringFormatter ); + formatterFactory.registerFormatter( StringFormatter, StringValue.TYPE ); assert.equal( formatterFactory.getFormatter( StringValue.TYPE ), @@ -100,7 +113,7 @@ title: 'Empty ValueFormatterFactory', register: [], expect: [ - [ StringValue.TYPE, null ], + [ StringValue, null ], [ stringType, null ] ] }, @@ -108,12 +121,12 @@ title: 'Factory with formatter for string DataValue which is also suitable for string ' + 'DataType', register: [ - [ StringValue.TYPE, StringFormatter ] + [ StringValue, StringFormatter ] ], expect: [ - [ StringValue.TYPE, StringFormatter ], + [ StringValue, StringFormatter ], [ stringType, StringFormatter ], // data type uses value type - [ UnknownValue.TYPE, null ], + [ UnknownValue, null ], [ numberType, null ] ] }, @@ -124,7 +137,7 @@ [ stringType, StringFormatter ] ], expect: [ - [ StringValue.TYPE, null ], + [ StringValue, null ], [ stringType, StringFormatter ] ] }, @@ -132,24 +145,24 @@ title: 'Factory with two formatters: For DataValue and for DataType using that ' + 'DataValue type', register: [ - [ StringValue.TYPE, StringFormatter ], + [ StringValue, StringFormatter ], [ stringType, StringFormatter ] ], expect: [ - [ StringValue.TYPE, StringFormatter ], + [ StringValue, StringFormatter ], [ stringType, StringFormatter ], - [ UnknownValue.TYPE, null ] + [ UnknownValue, null ] ] }, { title: 'Factory with two formatters for two different DataValue types', register: [ - [ StringValue.TYPE, StringFormatter ], - [ UnknownValue.TYPE, NullFormatter ] + [ StringValue, StringFormatter ], + [ UnknownValue, NullFormatter ] ], expect: [ - [ StringValue.TYPE, StringFormatter ], - [ UnknownValue.TYPE, NullFormatter ], + [ StringValue, StringFormatter ], + [ UnknownValue, NullFormatter ], [ numberType, null ] ] } @@ -174,9 +187,17 @@ // Register ValueFormatters as per definition: $.each( toRegister, function( i, registerPair ) { var purpose = registerPair[0], - formatter = registerPair[1]; - - formatterFactory.registerFormatter( purpose, formatter ); + Formatter = registerPair[1]; + + if( purpose instanceof DataTypeMock ) { + formatterFactory.registerFormatter( + Formatter, + purpose.getDataValueType(), + purpose.getId() + ); + } else { + formatterFactory.registerFormatter( Formatter, purpose.TYPE ); + } assert.ok( true, @@ -187,13 +208,22 @@ // Check for expected conditions: $.each( toExpect, function( i, expectPair ) { var purpose = expectPair[0], - formatter = expectPair[1]; + Formatter = expectPair[1], + RetrievedFormatter; + + if( purpose instanceof DataTypeMock ) { + RetrievedFormatter = formatterFactory.getFormatter( + purpose.getDataValueType(), purpose.getId() + ); + } else { + RetrievedFormatter = formatterFactory.getFormatter( purpose.TYPE ); + } assert.strictEqual( - formatterFactory.getFormatter( purpose ), - formatter, + RetrievedFormatter, + Formatter, 'Requesting formatter for ' + getTypeInfo( purpose ) + - ( formatter !== null ? ' returns expected formatter' : ' returns null' ) + ( Formatter !== null ? ' returns expected formatter' : ' returns null' ) ); } ); } @@ -204,4 +234,4 @@ valueFormatterFactoryRegistrationTest( assert, params.register, params.expect ); } ); -}( jQuery, QUnit, dataTypes, dataValues, valueFormatters ) ); +}( jQuery, QUnit, dataValues, valueFormatters ) ); diff --git a/js/tests/ValueParsers/ValueParserFactory.tests.js b/js/tests/ValueParsers/ValueParserFactory.tests.js index b964e7c..6c02557 100644 --- a/js/tests/ValueParsers/ValueParserFactory.tests.js +++ b/js/tests/ValueParsers/ValueParserFactory.tests.js @@ -2,33 +2,42 @@ * @licence GNU GPL v2+ * @author H. Snater < mediawiki@snater.com > */ -( function( $, QUnit, dt, dv, vp ) { +( function( $, QUnit, dv, vp ) { 'use strict'; + var DataTypeMock = function( dataTypeId, DataValue ) { + this._dataTypeId = dataTypeId; + this._dataValueType = DataValue.TYPE; + }; + $.extend( DataTypeMock.prototype, { + getId: function() { + return this._dataTypeId; + }, + getDataValueType: function() { + return this._dataValueType; + } + } ); + /** * Returns a descriptive string to be used as id when registering a ValueParser in a * ValueParserFactory. * - * @param {dataTypes.DataType|dataValues.DataValue|Function} purpose + * @param {DataTypeMock|Function} purpose * @return {string} */ function getTypeInfo( purpose ) { - if( purpose instanceof dt.DataType ) { + if( purpose instanceof DataTypeMock ) { return 'DataType with data value type "' + purpose.getDataValueType() + '"'; } - if( purpose instanceof dv.DataValue ) { - return 'DataValue instance of type "' + purpose.getType() + '"'; - } - // DataValue constructor: return 'constructor for DataValue of type "' + purpose.TYPE + '"'; } var StringValue = dv.StringValue, BoolValue = dv.BoolValue, - stringType = new dt.DataType( 'somestringtype', StringValue ), - numberType = new dt.DataType( 'somenumbertype', dv.NumberValue ), - StringParser = vp.StringParser, - BoolParser = vp.BoolParser; + stringType = new DataTypeMock( 'somestringtype', StringValue ), + numberType = new DataTypeMock( 'somenumbertype', dv.NumberValue ), + stringParser = vp.StringParser, + boolParser = vp.BoolParser; QUnit.module( 'valueParsers.ValueParserFactory' ); @@ -46,19 +55,19 @@ assert.throws( function() { - parserFactory.registerParser( StringValue.TYPE, 'invalid' ); + parserFactory.registerParser( 'invalid', StringValue.TYPE ); }, 'Failed trying to register an invalid parser constructor.' ); assert.throws( function() { - parserFactory.register( 'invalid', StringParser ); + parserFactory.register( stringParser, 'invalid' ); }, 'Failed trying to register a parser with an invalid purpose.' ); - parserFactory.registerParser( StringValue.TYPE, StringParser ); + parserFactory.registerParser( stringParser, StringValue.TYPE ); assert.throws( function() { @@ -68,30 +77,6 @@ ); } ); - QUnit.test( 'Return default parser on getParser()', function( assert ) { - var parserFactory = new vp.ValueParserFactory( BoolParser ); - - assert.equal( - parserFactory.getParser( StringValue.TYPE ), - BoolParser, - 'Returning default parser if no parser is registered for a specific data value.' - ); - - parserFactory.registerParser( StringValue.TYPE, StringParser ); - - assert.equal( - parserFactory.getParser( StringValue.TYPE ), - StringParser, - 'Returning specific parser if a parser is registered for a specific data value.' - ); - - assert.equal( - parserFactory.getParser( BoolValue.TYPE ), - BoolParser, - 'Still returning default parser if no parser is registered for a specific data value.' - ); - } ); - // Tests regarding registration of parsers: /** @@ -103,7 +88,7 @@ title: 'Empty ValueParserFactory', register: [], expect: [ - [ StringValue.TYPE, null ], + [ StringValue, null ], [ stringType, null ] ] }, @@ -111,12 +96,12 @@ title: 'Factory with parser for string DataValue which is also suitable for string ' + 'DataType', register: [ - [ StringValue.TYPE, StringParser ] + [ StringValue, stringParser ] ], expect: [ - [ StringValue.TYPE, StringParser ], - [ stringType, StringParser ], // data type uses value type - [ BoolValue.TYPE, null ], + [ StringValue, stringParser ], + [ stringType, stringParser ], // data type uses value type + [ BoolValue, null ], [ numberType, null ] ] }, @@ -124,35 +109,35 @@ title: 'Factory for string DataType. String DataValue can\'t use this potentially more ' + 'specialized parser', register: [ - [ stringType, StringParser ] + [ stringType, stringParser ] ], expect: [ - [ StringValue.TYPE, null ], - [ stringType, StringParser ] + [ StringValue, null ], + [ stringType, stringParser ] ] }, { title: 'Factory with two parsers: For DataValue and for DataType using that DataValue ' + 'type', register: [ - [ StringValue.TYPE, StringParser ], - [ stringType, StringParser ] + [ StringValue, stringParser ], + [ stringType, stringParser ] ], expect: [ - [ StringValue.TYPE, StringParser ], - [ stringType, StringParser ], - [ BoolValue.TYPE, null ] + [ StringValue, stringParser ], + [ stringType, stringParser ], + [ BoolValue, null ] ] }, { title: 'Factory with two parsers for two different DataValue types', register: [ - [ StringValue.TYPE, StringParser ], - [ BoolValue.TYPE, BoolParser ] + [ StringValue, stringParser ], + [ BoolValue, boolParser ] ], expect: [ - [ StringValue.TYPE, StringParser ], - [ BoolValue.TYPE, BoolParser ], + [ StringValue, stringParser ], + [ BoolValue, boolParser ], [ numberType, null ] ] } @@ -177,9 +162,13 @@ // Register ValueParsers as per definition: $.each( toRegister, function( i, registerPair ) { var purpose = registerPair[0], - parser = registerPair[1]; + Parser = registerPair[1]; - parserFactory.registerParser( purpose, parser ); + if( purpose instanceof DataTypeMock ) { + parserFactory.registerParser( Parser, purpose.getDataValueType(), purpose.getId() ); + } else { + parserFactory.registerParser( Parser, purpose.TYPE ); + } assert.ok( true, @@ -190,13 +179,22 @@ // Check for expected conditions: $.each( toExpect, function( i, expectPair ) { var purpose = expectPair[0], - parser = expectPair[1]; + Parser = expectPair[1], + RetrievedParser; + + if( purpose instanceof DataTypeMock ) { + RetrievedParser = parserFactory.getParser( + purpose.getDataValueType(), purpose.getId() + ); + } else { + RetrievedParser = parserFactory.getParser( purpose.TYPE ); + } assert.strictEqual( - parserFactory.getParser( purpose ), - parser, + RetrievedParser, + Parser, 'Requesting parser for ' + getTypeInfo( purpose ) + - ( parser !== null ? ' returns expected parser' : ' returns null' ) + ( Parser !== null ? ' returns expected parser' : ' returns null' ) ); } ); } @@ -207,4 +205,4 @@ valueParserFactoryRegistrationTest( assert, params.register, params.expect ); } ); -}( jQuery, QUnit, dataTypes, dataValues, valueParsers ) ); +}( jQuery, QUnit, dataValues, valueParsers ) ); From 15cbe9f6ece81609ea69bd263243e8406fd26981 Mon Sep 17 00:00:00 2001 From: snaterlicious Date: Tue, 14 Jan 2014 17:14:52 +0100 Subject: [PATCH 2/3] Switched to using dedicated registerParser/registerFormatter functions --- .../ValueFormatters/ValueFormatterFactory.js | 30 +---- .../ValueFormatters/mw.ext.valueFormatters.js | 2 +- js/src/ValueParsers/ValueParserFactory.js | 29 +--- js/src/ValueParsers/mw.ext.valueParsers.js | 8 +- .../ValueFormatterFactory.tests.js | 72 +++++++--- .../ValueParsers/ValueParserFactory.tests.js | 124 ++++++++++++++---- 6 files changed, 160 insertions(+), 105 deletions(-) diff --git a/js/src/ValueFormatters/ValueFormatterFactory.js b/js/src/ValueFormatters/ValueFormatterFactory.js index d8e9bb6..5eb1331 100644 --- a/js/src/ValueFormatters/ValueFormatterFactory.js +++ b/js/src/ValueFormatters/ValueFormatterFactory.js @@ -37,32 +37,6 @@ */ _formattersForDataValueTypes: null, - /** - * Registers a formatter. - * @since 0.1 - * - * @param {Function} Formatter ValueFormatter constructor. - * @param {string} dataValueType - * @param {string} [dataTypeId] - * - * @throws {Error} if the formatter constructor is invalid. - * @throws {Error} no proper purpose is provided. - */ - registerFormatter: function( Formatter, dataValueType, dataTypeId ) { - if( !$.isFunction( Formatter ) ) { - throw new Error( 'Invalid ValueFormatter constructor' ); - } - - if( typeof dataTypeId === 'string' ) { - this._registerDataTypeFormatter( Formatter, dataTypeId ); - } else if( typeof dataValueType === 'string' ) { - this._registerDataValueFormatter( Formatter, dataValueType ); - } else { - throw new Error( 'No sufficient purpose provided what to register the formatter ' - + 'for' ); - } - }, - /** * Registers a formatter for a certain data type. * @since 0.1 @@ -72,7 +46,7 @@ * * @throws {Error} if a formatter for the specified dataType object is registered already. */ - _registerDataTypeFormatter: function( Formatter, dataTypeId ) { + registerDataTypeFormatter: function( Formatter, dataTypeId ) { assertIsValueFormatterConstructor( Formatter ); if( this._formattersForDataTypes[dataTypeId] ) { @@ -92,7 +66,7 @@ * * @throws {Error} if a formatter for the specified DataValue type is registered already. */ - _registerDataValueFormatter: function( Formatter, dataValueType ) { + registerDataValueFormatter: function( Formatter, dataValueType ) { assertIsValueFormatterConstructor( Formatter ); if( this._formattersForDataValueTypes[dataValueType] ) { diff --git a/js/src/ValueFormatters/mw.ext.valueFormatters.js b/js/src/ValueFormatters/mw.ext.valueFormatters.js index 7b9e029..8955f0c 100644 --- a/js/src/ValueFormatters/mw.ext.valueFormatters.js +++ b/js/src/ValueFormatters/mw.ext.valueFormatters.js @@ -11,7 +11,7 @@ var valueFormatterProvider = new vf.ValueFormatterFactory( vf.NullFormatter ); - valueFormatterProvider.registerFormatter( + valueFormatterProvider.registerDataValueFormatter( vf.StringFormatter, dv.StringValue.TYPE ); diff --git a/js/src/ValueParsers/ValueParserFactory.js b/js/src/ValueParsers/ValueParserFactory.js index 5235e0f..c8f74f2 100644 --- a/js/src/ValueParsers/ValueParserFactory.js +++ b/js/src/ValueParsers/ValueParserFactory.js @@ -37,31 +37,6 @@ */ _parsersForDataValueTypes: null, - /** - * Registers a parser. - * @since 0.1 - * - * @param {Function} Parser ValueParser constructor. - * @param {string} dataValueType - * @param {string} [dataTypeId] - * - * @throws {Error} if the parser constructor is invalid. - * @throws {Error} no proper purpose is provided. - */ - registerParser: function( Parser, dataValueType, dataTypeId ) { - if( !$.isFunction( Parser ) ) { - throw new Error( 'Invalid ValueParser constructor' ); - } - - if( typeof dataTypeId === 'string' ) { - this._registerDataTypeParser( Parser, dataTypeId ); - } else if( typeof dataValueType === 'string' ) { - this._registerDataValueParser( Parser, dataValueType ); - } else { - throw new Error( 'No sufficient purpose to register the parser for provided' ); - } - }, - /** * Registers a parser for a certain data type. * @since 0.1 @@ -71,7 +46,7 @@ * * @throws {Error} if a parser for the specified dataType object is registered already. */ - _registerDataTypeParser: function( Parser, dataTypeId ) { + registerDataTypeParser: function( Parser, dataTypeId ) { assertIsValueParserConstructor( Parser ); if( this._parsersForDataTypes[dataTypeId] ) { @@ -90,7 +65,7 @@ * * @throws {Error} if a parser for the specified DataValue type is registered already. */ - _registerDataValueParser: function( Parser, dataValueType ) { + registerDataValueParser: function( Parser, dataValueType ) { assertIsValueParserConstructor( Parser ); if( this._parsersForDataValueTypes[dataValueType] ) { diff --git a/js/src/ValueParsers/mw.ext.valueParsers.js b/js/src/ValueParsers/mw.ext.valueParsers.js index 63df7f7..3b1b714 100644 --- a/js/src/ValueParsers/mw.ext.valueParsers.js +++ b/js/src/ValueParsers/mw.ext.valueParsers.js @@ -11,22 +11,22 @@ var valueParserProvider = new vp.ValueParserFactory( vp.NullParser ); - valueParserProvider.registerParser( + valueParserProvider.registerDataValueParser( vp.GlobeCoordinateParser, dv.GlobeCoordinateValue.TYPE ); - valueParserProvider.registerParser( + valueParserProvider.registerDataValueParser( vp.QuantityParser, dv.QuantityValue.TYPE ); - valueParserProvider.registerParser( + valueParserProvider.registerDataValueParser( vp.StringParser, dv.StringValue.TYPE ); - valueParserProvider.registerParser( + valueParserProvider.registerDataValueParser( vp.TimeParser, dv.TimeValue.TYPE ); diff --git a/js/tests/ValueFormatters/ValueFormatterFactory.tests.js b/js/tests/ValueFormatters/ValueFormatterFactory.tests.js index 391f38f..c97c2b9 100644 --- a/js/tests/ValueFormatters/ValueFormatterFactory.tests.js +++ b/js/tests/ValueFormatters/ValueFormatterFactory.tests.js @@ -50,28 +50,55 @@ ); } ); - QUnit.test( 'Error handling', function( assert ) { + QUnit.test( 'registerDataTypeFormatter(): Error handling', function( assert ) { var formatterFactory = new vf.ValueFormatterFactory(); assert.throws( function() { - formatterFactory.registerFormatter( 'invalid', StringValue ); + formatterFactory.registerDataTypeFormatter( 'invalid', stringType.getId() ); }, 'Failed trying to register an invalid formatter constructor.' ); assert.throws( function() { - formatterFactory.register( StringFormatter, 'invalid' ); + formatterFactory.registerDataTypeFormatter( StringFormatter, 'invalid' ); }, 'Failed trying to register a formatter with an invalid purpose.' ); - formatterFactory.registerFormatter( StringFormatter, StringValue.TYPE ); + formatterFactory.registerDataTypeFormatter( StringFormatter, stringType.getId() ); assert.throws( function() { - formatterFactory.getFormatter( StringValue ); + formatterFactory.getFormatter( stringType.getId() ); + }, + 'Failed trying to get a formatter with an invalid purpose.' + ); + } ); + + QUnit.test( 'registerDataValueFormatter(): Error handling', function( assert ) { + var formatterFactory = new vf.ValueFormatterFactory(); + + assert.throws( + function() { + formatterFactory.registerDataValueFormatter( 'invalid', StringValue.TYPE ); + }, + 'Failed trying to register an invalid formatter constructor.' + ); + + assert.throws( + function() { + formatterFactory.registerDataValueFormatter( StringFormatter, 'invalid' ); + }, + 'Failed trying to register a formatter with an invalid purpose.' + ); + + formatterFactory.registerDataValueFormatter( StringFormatter, StringValue.TYPE ); + + assert.throws( + function() { + formatterFactory.getFormatter( StringValue.TYPE ); }, 'Failed trying to get a formatter with an invalid purpose.' ); @@ -86,7 +113,13 @@ 'Returning default formatter if no formatter is registered for a specific data value.' ); - formatterFactory.registerFormatter( StringFormatter, StringValue.TYPE ); + assert.equal( + formatterFactory.getFormatter( stringType.getDataValueType(), stringType.getId() ), + NullFormatter, + 'Returning default formatter if no formatter is registered for a specific data type.' + ); + + formatterFactory.registerDataValueFormatter( StringFormatter, StringValue.TYPE ); assert.equal( formatterFactory.getFormatter( StringValue.TYPE ), @@ -100,6 +133,13 @@ 'Still returning default formatter if no formatter is registered for a specific data ' + 'value.' ); + + assert.equal( + formatterFactory.getFormatter( numberType.getDataValueType(), numberType.getId() ), + NullFormatter, + 'Still returning default formatter if no formatter is registered for a specific data ' + + 'type.' + ); } ); // Tests regarding registration of formatters: @@ -175,7 +215,8 @@ * @param {QUnit.assert} assert * @param {Array[]} toRegister Array containing arrays where each one tells a * ValueFormatterFactory what formatters to register. The inner array has to consist out - * of two objects as ValueFormatterFactory.registerFormatter would take them. + * of two objects, a formatter constructor and a DataValue constructor or a DataTypeMock + * object. * @param {Array[]} toExpect Array containing arrays where each one states one expected * condition of the ValueFormatterFactory after registration of what is given in the * first parameter. Each inner array should contain a data type, data value or data value @@ -190,13 +231,9 @@ Formatter = registerPair[1]; if( purpose instanceof DataTypeMock ) { - formatterFactory.registerFormatter( - Formatter, - purpose.getDataValueType(), - purpose.getId() - ); + formatterFactory.registerDataTypeFormatter( Formatter, purpose.getId() ); } else { - formatterFactory.registerFormatter( Formatter, purpose.TYPE ); + formatterFactory.registerDataValueFormatter( Formatter, purpose.TYPE ); } assert.ok( @@ -230,8 +267,11 @@ QUnit .cases( valueFormatterFactoryRegistrationTestCases ) - .test( 'registerFormatter() & getFormatter() ', function( params, assert ) { - valueFormatterFactoryRegistrationTest( assert, params.register, params.expect ); - } ); + .test( + 'registerDataTypeFormatter() / registerDataValueFormatter() & getFormatter() ', + function( params, assert ) { + valueFormatterFactoryRegistrationTest( assert, params.register, params.expect ); + } + ); }( jQuery, QUnit, dataValues, valueFormatters ) ); diff --git a/js/tests/ValueParsers/ValueParserFactory.tests.js b/js/tests/ValueParsers/ValueParserFactory.tests.js index 6c02557..7850c49 100644 --- a/js/tests/ValueParsers/ValueParserFactory.tests.js +++ b/js/tests/ValueParsers/ValueParserFactory.tests.js @@ -33,11 +33,11 @@ } var StringValue = dv.StringValue, - BoolValue = dv.BoolValue, + UnknownValue = dv.UnknownValue, stringType = new DataTypeMock( 'somestringtype', StringValue ), numberType = new DataTypeMock( 'somenumbertype', dv.NumberValue ), - stringParser = vp.StringParser, - boolParser = vp.BoolParser; + StringParser = vp.StringParser, + NullParser = vp.NullParser; QUnit.module( 'valueParsers.ValueParserFactory' ); @@ -50,24 +50,24 @@ ); } ); - QUnit.test( 'Error handling', function( assert ) { + QUnit.test( 'registerDataTypeParser(): Error handling', function( assert ) { var parserFactory = new vp.ValueParserFactory(); assert.throws( function() { - parserFactory.registerParser( 'invalid', StringValue.TYPE ); + parserFactory.registerDataTypeParser( 'invalid', stringType.getId() ); }, 'Failed trying to register an invalid parser constructor.' ); assert.throws( function() { - parserFactory.register( stringParser, 'invalid' ); + parserFactory.registerDataTypeParser( StringParser, 'invalid' ); }, 'Failed trying to register a parser with an invalid purpose.' ); - parserFactory.registerParser( stringParser, StringValue.TYPE ); + parserFactory.registerDataTypeParser( StringParser, stringType.getId() ); assert.throws( function() { @@ -77,6 +77,69 @@ ); } ); + QUnit.test( 'registerDataValueParser(): Error handling', function( assert ) { + var parserFactory = new vp.ValueParserFactory(); + + assert.throws( + function() { + parserFactory.registerDataValueParser( 'invalid', StringValue.TYPE ); + }, + 'Failed trying to register an invalid parser constructor.' + ); + + assert.throws( + function() { + parserFactory.registerDataValueParser( StringParser, 'invalid' ); + }, + 'Failed trying to register a parser with an invalid purpose.' + ); + + parserFactory.registerDataValueParser( StringParser, StringValue.TYPE ); + + assert.throws( + function() { + parserFactory.getParser( StringValue ); + }, + 'Failed trying to get a parser with an invalid purpose.' + ); + } ); + + QUnit.test( 'Return default parser on getParser()', function( assert ) { + var parserFactory = new vp.ValueParserFactory( NullParser ); + + assert.equal( + parserFactory.getParser( StringValue.TYPE ), + NullParser, + 'Returning default parser if no parser is registered for a specific data value.' + ); + + assert.equal( + parserFactory.getParser( stringType.getDataValueType(), stringType.getId() ), + NullParser, + 'Returning default parser if no parser is registered for a specific data type.' + ); + + parserFactory.registerDataValueParser( StringParser, StringValue.TYPE ); + + assert.equal( + parserFactory.getParser( StringValue.TYPE ), + StringParser, + 'Returning specific parser if a parser is registered for a specific data value.' + ); + + assert.equal( + parserFactory.getParser( UnknownValue.TYPE ), + NullParser, + 'Still returning default parser if no parser is registered for a specific data value.' + ); + + assert.equal( + parserFactory.getParser( numberType.getDataValueType(), numberType.getId() ), + NullParser, + 'Still returning default parser if no parser is registered for a specific data type.' + ); + } ); + // Tests regarding registration of parsers: /** @@ -96,12 +159,12 @@ title: 'Factory with parser for string DataValue which is also suitable for string ' + 'DataType', register: [ - [ StringValue, stringParser ] + [ StringValue, StringParser ] ], expect: [ - [ StringValue, stringParser ], - [ stringType, stringParser ], // data type uses value type - [ BoolValue, null ], + [ StringValue, StringParser ], + [ stringType, StringParser ], // data type uses value type + [ UnknownValue, null ], [ numberType, null ] ] }, @@ -109,35 +172,35 @@ title: 'Factory for string DataType. String DataValue can\'t use this potentially more ' + 'specialized parser', register: [ - [ stringType, stringParser ] + [ stringType, StringParser ] ], expect: [ [ StringValue, null ], - [ stringType, stringParser ] + [ stringType, StringParser ] ] }, { title: 'Factory with two parsers: For DataValue and for DataType using that DataValue ' + 'type', register: [ - [ StringValue, stringParser ], - [ stringType, stringParser ] + [ StringValue, StringParser ], + [ stringType, StringParser ] ], expect: [ - [ StringValue, stringParser ], - [ stringType, stringParser ], - [ BoolValue, null ] + [ StringValue, StringParser ], + [ stringType, StringParser ], + [ UnknownValue, null ] ] }, { title: 'Factory with two parsers for two different DataValue types', register: [ - [ StringValue, stringParser ], - [ BoolValue, boolParser ] + [ StringValue, StringParser ], + [ UnknownValue, NullParser ] ], expect: [ - [ StringValue, stringParser ], - [ BoolValue, boolParser ], + [ StringValue, StringParser ], + [ UnknownValue, NullParser ], [ numberType, null ] ] } @@ -149,8 +212,8 @@ * * @param {QUnit.assert} assert * @param {Array[]} toRegister Array containing arrays where each one tells a ValueParserFactory - * what parsers to register. The inner array has to consist out of two objects as - * ValueParserFactory.registerParser would take them. + * what parsers to register. The inner array has to consist out of two objects, a parser + * constructor and a DataValue constructor or a DataTypeMock object. * @param {Array[]} toExpect Array containing arrays where each one states one expected * condition of the ValueParserFactory after registration of what is given in the first * parameter. Each inner array should contain a data type, data value or data value @@ -165,9 +228,9 @@ Parser = registerPair[1]; if( purpose instanceof DataTypeMock ) { - parserFactory.registerParser( Parser, purpose.getDataValueType(), purpose.getId() ); + parserFactory.registerDataTypeParser( Parser, purpose.getId() ); } else { - parserFactory.registerParser( Parser, purpose.TYPE ); + parserFactory.registerDataValueParser( Parser, purpose.TYPE ); } assert.ok( @@ -201,8 +264,11 @@ QUnit .cases( valueParserFactoryRegistrationTestCases ) - .test( 'registerParser() & getParser() ', function( params, assert ) { - valueParserFactoryRegistrationTest( assert, params.register, params.expect ); - } ); + .test( + 'registerDataTypeParser() / registerDataValueParser() & getParser() ', + function( params, assert ) { + valueParserFactoryRegistrationTest( assert, params.register, params.expect ); + } + ); }( jQuery, QUnit, dataValues, valueParsers ) ); From e3dba73817c235bc316c9d1c61bc80b0d8603a65 Mon Sep 17 00:00:00 2001 From: snaterlicious Date: Tue, 14 Jan 2014 17:35:34 +0100 Subject: [PATCH 3/3] Added parameter check --- js/src/ValueFormatters/ValueFormatterFactory.js | 11 +++++++++++ js/src/ValueParsers/ValueParserFactory.js | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/js/src/ValueFormatters/ValueFormatterFactory.js b/js/src/ValueFormatters/ValueFormatterFactory.js index 5eb1331..499e2ce 100644 --- a/js/src/ValueFormatters/ValueFormatterFactory.js +++ b/js/src/ValueFormatters/ValueFormatterFactory.js @@ -49,6 +49,10 @@ registerDataTypeFormatter: function( Formatter, dataTypeId ) { assertIsValueFormatterConstructor( Formatter ); + if( dataTypeId === undefined ) { + throw new Error( 'No proper data type id provided to register the formatter for' ); + } + if( this._formattersForDataTypes[dataTypeId] ) { throw new Error( 'Formatter for DataType "' + dataTypeId + '" is registered ' + 'already' ); @@ -64,11 +68,17 @@ * @param {Function} Formatter * @param {string} dataValueType * + * @throws {Error} if no data type id is specified. * @throws {Error} if a formatter for the specified DataValue type is registered already. */ registerDataValueFormatter: function( Formatter, dataValueType ) { assertIsValueFormatterConstructor( Formatter ); + if( dataValueType === undefined ) { + throw new Error( 'No proper data value type provided to register the formatter ' + + 'for' ); + } + if( this._formattersForDataValueTypes[dataValueType] ) { throw new Error( 'Formatter for DataValue type "' + dataValueType + '" is ' + 'registered already' ); @@ -86,6 +96,7 @@ * @param {string} [dataTypeId] * @return {Function|null} * + * @throws {Error} if no data value type is specified. * @throws {Error} if no proper purpose is provided to retrieve a formatter. */ getFormatter: function( dataValueType, dataTypeId ) { diff --git a/js/src/ValueParsers/ValueParserFactory.js b/js/src/ValueParsers/ValueParserFactory.js index c8f74f2..7124cd0 100644 --- a/js/src/ValueParsers/ValueParserFactory.js +++ b/js/src/ValueParsers/ValueParserFactory.js @@ -44,11 +44,16 @@ * @param {Function} Parser * @param {string} dataTypeId * + * @throws {Error} if no data type id is specified. * @throws {Error} if a parser for the specified dataType object is registered already. */ registerDataTypeParser: function( Parser, dataTypeId ) { assertIsValueParserConstructor( Parser ); + if( dataTypeId === undefined ) { + throw new Error( 'No proper data type id provided to register the parser for' ); + } + if( this._parsersForDataTypes[dataTypeId] ) { throw new Error( 'Parser for DataType "' + dataTypeId + '" is registered already' ); } @@ -63,11 +68,16 @@ * @param {Function} Parser * @param {string} dataValueType * + * @throws {Error} if no data value type is specified. * @throws {Error} if a parser for the specified DataValue type is registered already. */ registerDataValueParser: function( Parser, dataValueType ) { assertIsValueParserConstructor( Parser ); + if( dataValueType === undefined ) { + throw new Error( 'No proper data value type provided to register the parser for' ); + } + if( this._parsersForDataValueTypes[dataValueType] ) { throw new Error( 'Parser for DataValue type "' + dataValueType + '" is registered ' + 'already' );