views:

291

answers:

7

A nasty gotcha in javascript is forgetting to call new on a function meant to be instantiated, leading to this being bound to a different object (usually the global) instead of a fresh one. One workaround I read about is to check for it explicitly in the function-constructor using the following idiom:

function SomeConstructor(x, y, ...) {
    // check if `this` is created with new
    if ( !(this instanceof arguments.callee) )
        return new SomeConstructor(x, y, ...);
    // normal initialization code follows

Now new SomeConstructor(...) and SomeConstructor(...) are equivalent.

I'd like to simplify this by creating a wrapper function factory(fn) that does the first two lines and then delegates to the wrapped function fn. This would be used like:

SomeConstructor = factory(function (x, y, ...) {
    // normal initialization code follows
})

My first attempt was:

function factory(fn) {
  return function() {
    if ( !(this instanceof arguments.callee) ) {
      return new arguments.callee.apply(this, arguments);
    }
    fn.apply(this, arguments);
  }
}

but it fails with "Function.prototype.apply called on incompatible [object Object]". The second attempt was:

function factory(fn) {
  return function() {
    if ( !(this instanceof arguments.callee) ) {
      var tmp = new arguments.callee();
      arguments.callee.apply(tmp, arguments);
      return tmp;
    }
    fn.apply(this, arguments);
  }
}

This sort of works but it may call the wrapped function twice: once with no arguments (to create a new instance) and once with the passed arguments for the actual initialization. Apparently this is fragile and inefficient but I can't figure out a way to do it with a single call. Is this possible ?

EDIT: Based on bobince's approach, here's a similar one that does the trick:

function factory(init) {
    var run_init = true;
    function constr() {
        if ( !(this instanceof constr) ) {
            run_init = false;
            var tmp = new constr();
            run_init = true;
            init.apply(tmp, arguments);
            return tmp;
        }
    if (run_init)
        init.apply(this, arguments);
  }
  return constr;
}

As for whether this is something that should be encouraged or not, that's debatable. I come from a Python background and I think of new as just noise (Java) or wart (Javascript), but I may be missing something.

+2  A: 

I dislike your mixing of arguments.callee and the function's identifier. Also, you are dumbing down the original problem. You should have used apply to begin with so as not to make the helper (factory) function seem even better than it really is.

What should have been done to begin with:

function SomeConstructor(x, y, ...) {
    // check if `this` is created with new
    if ( !(this instanceof arguments.callee) )
        return new arguments.callee.apply (this, arguments);
    // normal initialization code follows

Another issue with factory is that it defeats the function's length property.

trinithis
+8  A: 

This simply encourages a bad-habit shortcut that relies far too heavily on the implementation of the class to "fix" the calling code.

If this is a problem, don't just let it slide, throw an error message.

richardtallent
`new` introduces an irregularity into otherwise uniformly functional code. it's not a value, if you want to stick it somewhere, you need to wrap it in a `function` expression. you're back at square one, except now you have repetitive code at every callsite. bad habit? silly example: `map(new_(Array), range(0, 10))` versus `map(function (i) { return new Array(i); }, range(0, 10))`
just somebody
A: 

the only thing that worked for me involves dumbing down the implementation. it's ugly but works (both with and without operator new):

var new_ = function (cls)
{
    var constructors = [
        function ()
        {
            return new cls();
        }
      , function ($0)
        {
            return new cls($0);
        }
      , function ($0, $1)
        {
            return new cls($0, $1);
        }
      , function ($0, $1, $2)
        {
            return new cls($0, $1, $2);
        }
      , // up to a chosen limit
    ];
    return function ()
    {
        return constructors[arguments.length].apply(
            this
          , arguments
        );
    }
}

edit to react to comments

I have way-below-average tolerance to repetitive code, and this code hurt to write, but the functions in constructors need to be separate if arguments.length is to mean something in the real constructor function. consider a variant with a single new wrapper:

var new_ = function (cls)
{
    // arbitrary limit
    var constructor = function ($0, $1, $2)
    {
        return new cls($0, $1, $2);
    };
    return function ()
    {
        return constructor.apply(
            this
          , arguments
        );
    }
}

var gen = new_(function ()
{
    print(
        arguments.length
      + " "
      + Array.prototype.toSource.call(arguments)
    );
});
gen("foo")               // 3 ["foo", undefined, undefined]
gen("foo", "bar")        // 3 ["foo", "bar", undefined]
gen("foo", "bar", "baz") // 3 ["foo", "bar", "baz"]

the parameter list can be arbitrarily wide, but arguments.length doesn't lie only in the special case.

I've been using this solution with the upper limit of 10 arguments for a few years, and I don't remember ever running into the limit. the risk that it'll ever happen is rather low: everybody knows that functions with many parameters are a no-no, and javascript has a better interface for the desired functionality: packing parameters into objects.

so, the only limit is the width of the parameter list, and this limit seems to be purely theoretical. other than that, it supports completely arbitrary constructors, so I'd say it's very general.

just somebody
Since it's valid to call functions with too many arguments, you could probably just get away with the longest fixed-size caller. Or just `function applynew(cls, a) { return new cls(a[0], a[1], a[2], a[3], a[4]); }`?
bobince
using only the longest form would break `arguments` in `cls`. `applynew` doesn't solve the problem, and the interface is ugly.
just somebody
-1: Not general.
trinithis
+3  A: 

Your "factory" function could be written in this way:

function factory(fn, /* arg1, arg2, ..., argn */) {
  var obj = new fn(), // Instantiate using 'new' to preserve the prototype chain
      args = Array.prototype.slice.call(arguments, 1); // remove fn argument
  fn.apply(obj, args); // apply the constructor again, with the right arguments
  return obj;
}

// Test usage:
function SomeConstructor (foo, bar) {
  this.foo = foo;
  this.bar = bar;
}
SomeConstructor.prototype.test = true;

var o = factory(SomeConstructor, 'foo', 'bar');
// will return: Object foo=foo bar=bar test=true, and
o instanceof SomeConstructor; // true

However, the new operator is not bad, I would not encourage you to avoid it, I would recommend you to stick with a proper naming convention, constructor functions identifiers in PascalCase, all other identifiers in camelCase, and also I highly recommend you to use JsLint it will help you to detect that kind of mistakes early.

CMS
Wouldn't function factory(fn, /* arg1, arg2, ..., argn */) { return new fn( Array.prototype.slice.call(arguments, 1) ); } be the shorter equivalent that would also avoid errors in constructors that may throw exceptions when called without arguments? Other than that, I completely agree with your notes on `new` not being bad.
Justin Johnson
@Justin: Your example is not equivalent, it will execute the constructor function passing only **one** argument, an Array object. Using the `SomeConstructor(foo, bar)` function I posted as an example, with your code, the argument `foo` will contain `['foo', 'bar']` and the argument `bar` will be `undefined`. The purpose of using `apply` is to pass the arguments separated, as expected by the function.
CMS
@Justin: BTW a couple of hours ago, I answered a question more in-depth about `apply`: http://stackoverflow.com/questions/1976015/how-to-implement-apply-pattern-in-javascript
CMS
I am aware of how `apply` works, but you're right, I overlooked that. Still, it doesn't address cases in which the `fn` constructor throws an error, for which I don't see any immediate solution. I suppose this goes back to your original recommendation.
Justin Johnson
Agreed with Justin. You need to make a 'new' function that does the same thing as new given a constructor, only it's a function so you can apply on it. Otherwise you could get into some hairy situations by calling it twice.
trinithis
@trinithis: Also agree, but however, at the end, I think that just using the `new` operator properly is worth more that having to make wrapper functions for constructors...
CMS
+2  A: 

You can pass a unique value into the constructor for the first call (with new) that signifies you don't want the initialiser called yet:

var _NOINIT= {};
function factory(init) {
    function constr() {
        if (!(this instanceof constr)) {
            var inst= new constr(_NOINIT);
            init.apply(inst, arguments);
            return inst;
        }
        if (arguments[0]!==_NOINIT)
            init.apply(this, arguments);
    }
    return constr;
}

Note I've used a named inline function for the constructor because arguments.callee will be going away in ECMAScript Fifth Edition's ‘strict’ mode.

However if you're using a class factory, I suggest making the initialiser function a member of the class, rather than being passed in. That way, you can subclass a base class and have the subclass inherit the initialiser, which is normal behaviour in class-based languages. eg.:

Function.prototype.makeSubclass= function() {
    function constr() {
        var that= this;
        if (!(this instanceof constr))
            that= new constr(_NOINIT);
        if (arguments[0]!==_NOINIT && '_init' in that)
            that._init.apply(that, arguments);
        return that;
    }

    if (this!==Object)
        constr.prototype= new this(_NOINIT);
    return constr;
};

var Shape= Object.makeSubclass();
Shape.prototype._init= function(x, y) {
    this.x= x;
    this.y= y;
};

var Point= Shape.makeSubclass();
// inherits initialiser(x, y), as no need for anything else in there

var Circle= Shape.makeSubclass()
Circle.prototype._init= function(x, y, r) {
    Shape.prototype._init.call(this, x, y);
    this.r= r;
};

Of course you don't have to put that into the Function prototype... it's a matter of taste, really. As is allowing constructors without new.

Personally I prefer to throw an error rather than silently make it work, to try to discourage bare-constructor-calling, since this is a mistake elsewhere and may make the code slightly less clear.

bobince
Thanks, this does the trick! Based on the first approach, I came up with a similar one that uses a closure and a boolean variable to control initialization (see update)
gsakkis
Yep, seems like a good hack!
bobince
A: 

If you are interested in dealing with the inability to use apply with new, one could use

Function.prototype.New = (function () {
  var fs = [];
  return function () {
    var f = fs [arguments.length];
    if (f) {
      return f.apply (this, arguments);
    }
    var argStrs = [];
    for (var i = 0; i < arguments.length; ++i) {
      argStrs.push ("a[" + i + "]");
    }
    f = new Function ("var a=arguments;return new this(" + argStrs.join () + ");");
    if (arguments.length < 100) {
      fs [arguments.length] = f;
    }
    return f.apply (this, arguments);
  };
}) ();

Example:

var foo = Foo.New.apply (null, argArray);
trinithis
Why did you answer twice?
Crescent Fresh
Because the answers are quite unrelated.
trinithis
+1  A: 

while 'new' is a good thing, and I don't endorse trying to do away with language features, check out this code I played with a while ago: (note, this is not a complete solution for you, but rather something to build into your code)

function proxy(obj)
{
    var usingNew = true;
    var obj_proxy = function()
    {
        if (usingNew)
            this.constructor_new.apply(this, arguments);
    };
    obj_proxy.prototype = obj.prototype;
    obj_proxy.prototype.constructor_new = obj.prototype.constructor;

    obj_proxy.createInstance = function()
    {
        usingNew = false;
        var instance = new obj_proxy();
        instance.constructor_new.apply(instance, arguments);
        usingNew = true;

        return instance;
    }

    return obj_proxy;
}

to test it out, create a function foo like this:

function foo(a, b) { this.a = a; }

and test it:

var foo1 = proxy(foo);

var test1 = new foo1(1);
alert(test1 instanceof foo);

var test2 = foo1.createInstance(2);
alert(test2 instanceof foo);

EDIT: removed some code not relevant for this.

Luke Schafer