Placing instance properties in prototype constructors, good or bad?

Question

I just finished reading this very good article on prototypal inheritance in JavaScript, but was surprised to see how vehemently the author was against having properties defined in prototypes.

A common mistake when creating objects for the prototype chain, from programmers that come from classical OOP anyway, is to define common properties high up in the chain because they exist for all instances. We feel the need to define the property as if the abstract object described an interface. Yet there is no point in defining in a prototype a property that will be present in objects that descend from it. Javascript is not like Java : you don't declare in the base objects variables that will be different to all instances of the descendants. You declare a variable only on the level where it will be defined.

Take the name property of our animals. Since every animal has a name, it's natural to consider this property as common to all, and define it in the common denominator which is the Animal prototype. The thing is, Animal has no name. A Dog instance has a name.

In Javascript, you cannot say an Animal has a name. Animal is an object, not a definition, even if we use it like so. And that object has no name property. Why then is name referred to in Animal's methods if Animal has no name? Because Animal is abstract : it is not intended to be used by itself. this, in Animal, will never refer to Animal. It will refer to whatever object descends from Animal, dino for example. And dino has a name.

If I have a very complex set of classes that have, dozens of properties in common. I don't see how it's better to duplicate those properties and the work that goes into setting them up on each instantiable derived class when the work can be done once in the base class, even if that base class was meant to be 'abstract'.

For instance:

function Analysis(args){
  args = args || {};
  // Extract supported init properties from args
  this.description = args.description;
  this.host = args.host;
  this.source = args.source;
  this.identifier = args.identifier;
  this.vendor = args.vendor;
  this.agent = args.agent;
  //etc...
}

function PortfolioAnalysis(args){
  Analysis.call(this, args);
  args = args || {};
  this.portfolio = args.portfolio;
  this.author = args.author;
  //etc...
}
PortfolioAnalysis.prototype = Object.create(Analysis.prototype);
PortfolioAnalysis.prototype.constructor = PortfolioAnalysis;

function TreatyAnalysis(args){
  Analysis.call(this, args);
  args = args || {};
  this.treaty = args.treaty;
  this.terms = args.terms;
  //etc...
}
TreatyAnalysis.prototype = Object.create(Analysis.prototype);
TreatyAnalysis.prototype.constructor = TreatyAnalysis;

//etc...

So the article is saying I should paste the initialization code for the properties description, host, source, etc. in each of the derived classes, and remove it from the base class.

I don't see why that's better, especially if there's a bunch of complex common logic around constructing these objects using those shared properties, what's so bad about defining them in the base class, and if it's so bad, is there a way around it that doesn't involve code duplication or having to define a separate '.init()' method?

Answer 1

I don't think you get prototypal inheritance yet. The author is not saying “don't put the initialization code in the base constructor”. What the author is saying is “don't put properties on the base prototype”. It's totally different.

So you are allowed to do what you are currently doing. It's totally fine. What you shouldn't do however is put default values of properties on the prototype as it might cause problems. For example consider:

function Foo() {}

Foo.prototype.data = []; // you shouldn't do this

var a = new Foo;
var b = new Foo;

a.data.push(0);

alert(JSON.stringify(b.data)); // [0]

This is why you shouldn't share properties on the prototype. We modified the value of a.data but since data is shared amongst all instances of Foo we also modified b.data. Hence an invariant was violated.

Think about it like this:

1. The properties defined on the current object are its public properties. They are not shared.
2. The properties defined on the prototype of the current object are static properties. They are shared amongst the instances of the prototype.

Hence it's alright to define static properties, like the count of all the instances, on the prototype. However it's not alright to define public properties on the prototype because it may cause problems like the one above.

Dr. Alex Rauschmayer explains this better: http://www.2ality.com/2013/09/data-in-prototypes.html

Your code is fine because the this in your constructors always point to the current object, not the prototype. Hence you are not defining any properties on the prototype.

I think you got confused because of constructors vs prototypes. Perhaps this blog post would elucidate your doubts: http://aaditmshah.github.io/why-prototypal-inheritance-matters/

Answer 2

So the article is saying I should paste the initialization code for the properties description, host, source, etc. in each of the derived classes, and remove it from the base class.

No. Your code is perfectly fine, exactly how it should be done.

And that article is saying that the properties like description, host etc should be placed on instances (like a new ThreatAnalysis(…), or even a new Analysis(…)), but not on Analysis.prototype - just what you are doing. There are some people who would "default", e.g. empty, identifiers etc on Analysis.prototype because they want to "declare" that every Analysis instance should have an identifier. That is rubbish, as the article explains.

To share your initialisation behaviour in the Analysis constructor is fine (as the article mentions, shared functions may be placed hight in the prototype chain). There's no need to inline it and make Analysis and empty object, even if it is abstract and will never be instantiated directly.