Derek Chiang
Derek Chiang

Reputation: 3410

What are the downsides of defining functions on prototype this way?

Usually people write code like this:

function Shape() {
  this.x = 0;
  this.y = 0;
}

Shape.prototype.move = function(x, y) {
  this.x += x;
  this.y += y;
}

However I was trying to come up with a way to define a function on a prototype without separating the function definition with the constructor. Here is what I got:

Object.prototype.method = function(name, func) {
  if (typeof(this.constructor.prototype[name]) === 'undefined') {
    this.constructor.prototype[name] = func
  }
}

This allows you to do something like:

function Shape() {
  this.x = 0;
  this.y = 0;

  this.method('move', function(x, y) {
    this.x += x;
    this.y += y;
  })
}

And also no matter how many times you create a shape, the function will only be defined once.

I'm aware that augmenting Object.prototype isn't considered a good practice. But other than that are there any downsides with this approach?

EDIT:

Johan brought up a good point; I should have made method not enumerable. Here is the revised version:

Object.defineProperty(Object.prototype, 'method', {
    value: function(name, func) {
        if (typeof(this.constructor.prototype[name]) === 'undefined') {
            this.constructor.prototype[name] = func
        }
    },
    enumerable: false
})

Upvotes: 4

Views: 682

Answers (4)

Aadit M Shah
Aadit M Shah

Reputation: 74204

Let's actually compare both the ways and see which one is faster: http://jsperf.com/traditional-oop-vs-derek-s-oop-variant

As you can see your method is much slower than the traditional method. The reasons are:

  1. Your constructor is doing more stuff than required. Hence if you create multiple instances then the extra cost of creating an instance adds up.
  2. As @Alxandr mentioned you're creating a new anonymous function for method every time you create a new instance for no good reason. It'll only be useful once after which it'll be a waste of processing power.
  3. You're calling a function to check whether the prototype of the constructor has a given method or not, and to add the method to the prototype if it doesn't. This seems unnecessary. You don't need to create a function to do this for you. IMHO the function call is just additional overhead.

Since you asked for criticism:

I'm aware that augmenting Object.prototype isn't considered a good practice. But other than that are there any downsides with this approach?

Beside being terribly slow your approach also suffers from:

  1. Being difficult to understand. You may find this approach intuitive. However a person reading your code would surely wonder what the function this.method does. They would need to read the definition of Object.prototype.method to fully comprehend your code.
  2. Being unintuitive. As I mentioned before it makes no sense to be defining prototype properties inside the constructor. It'll only be needed once after which it'll just become additional baggage. It's best to keep the constructor logic and the prototype properties separate.
  3. It may lead to unexpected behavior. As @basilikum pointed out if you never call the constructor then the prototype properties will never be set. This may lead to problems when you attempt to access a property on the prototype. For example, when inheriting properties from the prototype no properties will be inherited until the base constructor is called.

I believe your goal is to encapsulate both the constructor and the prototype properties into a single entity:

However I was trying to come up with a way to define a function on a prototype without separating the function definition with the constructor.

Is there an easy way to do this? Let's see, JavaScript is a prototypal object oriented language. Hence we should focus more on the prototype instead of the constructor.

The above diagram was taken from the following answer: https://stackoverflow.com/a/8096017/783743

This diagram shows us:

  1. Every constructor has a property called prototype which points to the prototype object of the constructor function.
  2. Every prototype has a property called constructor which points to the constructor function of the prototype object.
  3. We create an instance from a constructor function. However the instance actually inherits from the prototype, not the constructor.

This is very useful information. Traditionally we've always created a constructor function first and then we've set its prototype properties. However this information shows us that we may create a prototype object first and then define the constructor property on it instead.

For example, traditionally we may write:

function Shape() {
    this.x = 0;
    this.y = 0;
}

Shape.prototype.move = function(x, y) {
    this.x += x;
    this.y += y;
};

However using our newfound knowledge we may write the same thing as:

var shape = {
    constructor: function () {
        this.x = 0;
        this.y = 0;
    },
    move: function (x, y) {
        this.x += x;
        this.y += y;
    }
};

The information contained in both these examples is the same. However we need a little additional scaffolding to make the second example work. In particular we need to do:

var Shape = shape.constructor;
Shape.prototype = shape;

This is not a big issue. We simply create a function to do this for us:

function defclass(prototype) {
    var constructor = prototype.constructor;
    constructor.prototype = prototype;
    return constructor;
}

Now we can define Shape as follows:

var Shape = defclass({
    constructor: function () {
        this.x = 0;
        this.y = 0;
    },
    move: function (x, y) {
        this.x += x;
        this.y += y;
    }
});

As you can see encapsulation is easy to achieve in JavaScript. All you need to do is think sideways. Inheritance however is a different issue. You need to do a little more work to achieve inheritance.

Hope this helped.

Upvotes: 7

basilikum
basilikum

Reputation: 10528

Alxandr already said the most, but I'd like to point out another downside of your method.

The property move will only be there if you call the constructor at least once. This can be bad when it comes to inheritance. Take this for example:

function Shape() {
  this.x = 0;
  this.y = 0;

  this.method(this, 'move', function(x, y) {
    this.x += x;
    this.y += y;
  });
}
function Triangle() {
}

Triangle.prototype = Object.create(Shape.prototype);

var t = new Triangle();
t.move(); //gives you an error because move is not defined

As you see, you have never called the constructor of Shape and therefor move isn't there yet. Normally you would call the Shape constructor inside of the Triangle constructor and you code would work fine, but there might be situation where you don't want to do this.

So I would advice to just stick to the original pattern. It is actually not that ugly when you get used to it.

Upvotes: 1

Alxandr
Alxandr

Reputation: 12423

First off, you state that "the function will only be defined once", however, this is not true AFAIK. Every time you construct a new Shape, the anonymous function

function(x,y) { this.x += x; this.y += y }

get's redefined (unless the compiler/interpreter is smart enough to optimize this away as it isn't used). It then get's passed to the method Object.prototype.method, which chooses to ignore it (becuase the method was already defined), and is simply left in memory to be garbage-collected. Running this in a loop (if the compiler/interpreter isn't able to optimize it away), would result in a lot of functions being created for no good reason.

Also, as Johan stated, littering in Object.prototype may cause other code to stop working because suddenly for(x in anything) will show your new function method.

If you would like to "group" the declaration of the constructor, and it's methods together, you could rather create something like this:

function mangle(ctor, methods) {
    for(x in methods) {
        if(methods.hasOwnProperty(x)) {
            ctor.prototype[x] = methods[x];
        }
    }
    return ctor;
}

which you'd then call like this:

var Shape = mangle(function() {
    // moved default-values to prototype
    // so they don't have to be created
    // if they aren't set to something
    // else than the default.
}, {
    x: 0,
    y: 0,
    move: function(x, y) {
        this.x += x;
        this.y += y;
    }
});

Or you could just stick to the normal design, or use a "class"-providing library like Prototype or MooTools. There are a lot of options to choose from.

Upvotes: 3

Johan Bouveng
Johan Bouveng

Reputation: 565

If you, or someone in the future trying loop over an object that is extended, the results might be unpredictable and break since the object now contains unknown properties. A little unforeseen gotcha!

Upvotes: 0

Related Questions