Inheritance problem in javascript

Question

I have the following class hierarchy:

Storage
- Collection
  - EixoCollection
  - OfertaCollection

And I have the following code:

var ofertaCollection = new OfertaCollection();
var eixoCollection = new EixoCollection();

ofertaCollection.set('mykey', 'myvalue');
alert(eixoCollection.get('mykey')); // it returns 'myvalue', should return nothing

The problem is that ofertaCollection and eixoCollection have properties referencing each other.

Follow the classes:

/**
 * Storage
 * 
 * @returns {Storage}
 */
function Storage(){

    this.storage = []; // itens that have a key

    // sets key
    this.set = function(key, value){
        this.storage[key] = value;
    }

    // gets key
    this.get = function(key){
        return this.storage[key];
    }
}

/**
 * Collection
 * 
 * @returns {Collection}
 */
function Collection(){

}
Collection.prototype = new Storage();

/**
 * EixoCollection
 * 
 * @returns {EixoCollection}
 */
function EixoCollection(){
}
EixoCollection.prototype = new Collection();

/**
 * OfertaCollection
 * 
 * @returns {OfertaCollection}
 */
function OfertaCollection(){
}
OfertaCollection.prototype = new Collection();

What is the problem?

Why do you even have the Collection class at all? Looks like serious bloat to me... all you've done is wrapped the Storage class. — Brian Driscoll
– Brian Driscoll, Commented Jun 6, 2011 at 19:30

davin · Accepted Answer · 2011-06-06 20:18:04Z

Firstly, I suggest two changes in your code (that the examples will use): this.storage should be an object {}, since it looks like you never use it as an array. Also, get and set should be in the prototype, otherwise a new instance of those methods will be created for every instance object (unless a smart compiler optimises them, but we won't assume as much).

Solution 1: you could perform quasi-inheritance, whereby only your storage methods are given to inheriting classes, but not the storage object:

function Storage(){}
storage.prototype = {
    get: function(key){
        return this.storage[key];
    },
    set: function(key, value){
        this.storage[key] = value;
    }
};

function Collection(){
    this.storage = {};
}
Collection.prototype = Storage.prototype; // quasi-inheritance

function EixoCollection(){}
EixoCollection.prototype = new Collection();

function OfertaCollection(){}
OfertaCollection.prototype = new Collection();

var ofertaCollection = new OfertaCollection();
var eixoCollection = new EixoCollection();

ofertaCollection.set('mykey', 'myvalue');
alert(eixoCollection.get('mykey')); // undefined

Solution 2: Real inheritance, but instantiate a new store for every collection, so during the prototype lookup the function finds the local store. This is identical to the above, but the inheritance would be as before. So I'll just replace the line that's different:

Collection.prototype = new Storage(); // real inheritance

The problem with both of these implementations is that they require the inheriting class to do two things, both inherit the methods, and instantiate a store. Not pretty.

The easy alternative, and maybe the most intuitive one is to make each use of Storage a composite object, and not an inherited one. A collection has an internal storage, and some extra functionalities, so it fulfils the has-a mnemonic making composition a valid candidate.

Jeremy · Accepted Answer · 2011-06-06 19:51:31Z

1

The problem is that both of your collection objects share the same Storage object.

var c1 = new OfteraCollection().prototype.prototype;
var c2 = new ExioCollection().prototype.prototype;
console.log(c1 === c2); // true

To remedy this, you could ditch Collection all together and have each one of your collection objects create their own Storage as their prototype.

edited Jun 6, 2011 at 19:51

answered Jun 6, 2011 at 19:43

Jeremy

22.5k4 gold badges71 silver badges82 bronze badges

4 Comments

Jeremy Over a year ago

I amended my answer, but I am still curious. Why are you doing this? Do you have other methods for your objects? From what I see, all you really need to do is say var coll = new Object(); coll[key] = value;

lucaswxp Over a year ago

Yes, I have. I only show that way to minify the code =). And I dont understand the amended answer, could you give a example? (newbie in js =p)

Jeremy Over a year ago

I can't right now, but read this in the meantime: javascript.crockford.com/prototypal.html

davin Over a year ago

Not sure what you mean by "create their own Storage as their prototype." If all you did was remove the Collection layer, then each type (if it's instantiating its prototype as new Storage()) would still suffer from the shared memory issue, and multiple instances of a single type wouldn't be independent.

nrabinowitz · Accepted Answer · 2011-06-06 21:35:39Z

1

Here's the very quick fix - change:

function Collection(){
   // assuming you're doing some initialization here, referencing "this"
}
Collection.prototype = new Storage();

to

function Collection(){
    var that = new Storage();
    // initialize whatever's necessary, using "that"
    return that;
}

or, if you aren't doing any initialization, just:

function Collection(){
    return new Storage();
}

Working example here.

This turns Collection into a Storage factory, which prevents reusing the same Storage objects in the descendants.

answered Jun 6, 2011 at 21:35

nrabinowitz

55.7k10 gold badges152 silver badges169 bronze badges

Collectives™ on Stack Overflow

Inheritance problem in javascript

3 Answers 3

Comments

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related