ve.Factory: No need for a-b-c workaround, createObject and apply.

This works just fine, as also previously tested/proven by
ve.cloneObject, which uses the same concept of creating an object
identical to what invoking the constructor with "new" would do,
but without invoking the constructor function (which has side-
effects).

Except in this case we do invoke the constructor function, but
we can't use new in ve.Factory because of the arbitrary number of
arguments.

Added a test to assert that 3+ arguments and that instanceof
work as expected.

Change-Id: If0add3da7475886e476900044acda2ba7d01fb11

modules/ve/test/ve.Factory.test.js

@@ -9,10 +9,11 @@ QUnit.module( 've.Factory' );
 
 /* Stubs */
 
-ve.FactoryObjectStub = function VeFactoryObjectStub( a, b, c ) {
+ve.FactoryObjectStub = function VeFactoryObjectStub( a, b, c, d ) {
 	this.a = a;
 	this.b = b;
 	this.c = c;
+	this.d = d;
 };
 
 /* Tests */
@@ -24,24 +25,36 @@ QUnit.test( 'register', 1, function ( assert ) {
 			factory.register( 'factory-object-stub', 'not-a-function' );
 		},
 		Error,
-		'throws an exception when trying to register a non-function value as a constructor'
+		'Throws an exception when trying to register a non-function value as a constructor'
 	);
 } );
 
-QUnit.test( 'create', 2, function ( assert ) {
-	var factory = new ve.Factory();
+QUnit.test( 'create', 3, function ( assert ) {
+	var obj,
+		factory = new ve.Factory();
+
 	assert.throws(
 		function () {
 			factory.create( 'factory-object-stub', 23, 'foo', { 'bar': 'baz' } );
 		},
 		Error,
-		'throws an exception when trying to create a object of an unregistered type'
+		'Throws an exception when trying to create a object of an unregistered type'
 	);
+
 	factory.register( 'factory-object-stub', ve.FactoryObjectStub );
+
+	obj = factory.create( 'factory-object-stub', 16, 'foo', { 'baz': 'quux' }, 5 );
+
 	assert.deepEqual(
-		factory.create( 'factory-object-stub', 16, 'foo', { 'baz': 'quux' } ),
-		new ve.FactoryObjectStub( 16, 'foo', { 'baz': 'quux' } ),
-		'creates objects of a registered type and passes through arguments'
+		obj,
+		new ve.FactoryObjectStub( 16, 'foo', { 'baz': 'quux' }, 5 ),
+		'Creates an object of the registered type and passes through arguments'
+	);
+
+	assert.strictEqual(
+		obj instanceof ve.FactoryObjectStub,
+		true,
+		'Creates an object that is an instanceof the registered constructor'
 	);
 } );
 

modules/ve/ve.Factory.js

@@ -52,22 +52,31 @@ ve.Factory.prototype.register = function ( type, constructor ) {
  * Type is used to look up the constructor to use, while all additional arguments are passed to the
  * constructor directly, so leaving one out will pass an undefined to the constructor.
  *
- * WARNING: JavaScript does not allow using new and .apply together, so we just pass through 3
- * arguments here since we know we only need that many at this time. If we need more in the future
- * we should change this to suit that use case. Because of undefined pass through, there's no harm
- * in adding more.
- *
  * @method
- * @param {String} type Object type
- * @param {Mixed} [...] Up to 3 additional arguments to pass through to the constructor
- * @returns {Object} The new object
+ * @param {string} type Object type.
+ * @param {mixed} [...] Arguments to pass to the constructor.
+ * @returns {Object} The new object.
  * @throws 'Unknown object type'
  */
-ve.Factory.prototype.create = function ( type, a, b, c ) {
-	if ( type in this.registry ) {
-		return new this.registry[type]( a, b, c );
+ve.Factory.prototype.create = function ( type ) {
+	var args, obj,
+		constructor = this.registry[type];
+
+	if ( constructor === undefined ) {
+		throw new Error( 'Unknown object type: ' + type );
 	}
-	throw new Error( 'Unknown object type: ' + type );
+
+	// Convert arguments to array and shift the first argument (type) off
+	args = Array.prototype.slice.call( arguments, 1 );
+
+	// We can't use the "new" operator with .apply directly because apply needs a
+	// context. So instead just do what "new" does: Create an object that inherits from
+	// the constructor's prototype (which also makes it an "instanceof" the constructor),
+	// then invoke the constructor with the object as context, and return it (ignoring
+	// the constructor's return value).
+	obj = ve.createObject( constructor.prototype );
+	constructor.apply( obj, args );
+	return obj;
 };
 
 /**