CODEWITHSUNDEEP
javascriptprototypedecorator
F4b
F4b

Reputation: 95

Javascript Decorator Pattern by Addy Osmani

Given this code:

// Constructor.
var Interface = function (name, methods) {
        if (arguments.length != 2) {
            throw new Error("Interface constructor called with " + arguments.length + "arguments, but expected exactly 2.");
        }
        this.name = name;
        this.methods = [];
        for (var i = 0, len = methods.length; i < len; i++) {
            if (typeof methods[i] !== 'string') {
                throw new Error("Interface constructor expects method names to be " + "passed in as a string.");
            }
            this.methods.push(methods[i]);
        }
    };


// Static class method.
Interface.ensureImplements = function (object) {
    if (arguments.length < 2) {
        throw new Error("Function Interface.ensureImplements called with " + arguments.length + "arguments, but expected at least 2.");
    }
    for (var i = 1, len = arguments.length; i < len; i++) {
        var interface = arguments[i];
        if (interface.constructor !== Interface) {
            throw new Error("Function Interface.ensureImplements expects arguments" + "two and above to be instances of Interface.");
        }
        for (var j = 0, methodsLen = interface.methods.length; j < methodsLen; j++) {
            var method = interface.methods[j];
            if (!object[method] || typeof object[method] !== 'function') {
                throw new Error("Function Interface.ensureImplements: object " + "does not implement the " + interface.name + " interface. Method " + method + " was not found.");
            }
        }
    }
};


function extend( a, b ){
    for( var key in b )
        if( b.hasOwnProperty(key) )
            a[key] = b[key];
    return a;
}




var Macbook = new Interface( "Macbook",
  ["addEngraving",
  "addParallels",
  "add4GBRam",
  "add8GBRam",
  "addCase"]); /* Returns an object that stores the name of the Interface and the array of methods (pushed) */

// A Macbook Pro might thus be represented as follows:
var MacbookPro = function(){
    // implements Macbook
};

MacbookPro.prototype = {
    addEngraving: function(){
    },
    addParallels: function(){
    },
    add4GBRam: function(){
    },
    add8GBRam:function(){
    },
    addCase: function(){
    },
    getPrice: function(){
      // Base price
      return 900.00;
    }
};


var MacbookDecorator = function( macbook ){

    Interface.ensureImplements( macbook, Macbook );
    this.macbook = macbook;

};

MacbookDecorator.prototype = {
    addEngraving: function(){
        return this.macbook.addEngraving();
    },
    addParallels: function(){
        return this.macbook.addParallels();
    },
    add4GBRam: function(){
        return this.macbook.add4GBRam();
    },
    add8GBRam:function(){
        return this.macbook.add8GBRam();
    },
    addCase: function(){
        return this.macbook.addCase();
    },
    getPrice: function(){
        return this.macbook.getPrice();
    }
};


var CaseDecorator = function( macbook ){
   this.macbook = macbook;
};

// Let's now extend (decorate) the CaseDecorator
// with a MacbookDecorator
extend( CaseDecorator, MacbookDecorator );

CaseDecorator.prototype.addCase = function(){
    return this.macbook.addCase() + "Adding case to macbook";
};

CaseDecorator.prototype.getPrice = function(){
    return this.macbook.getPrice() + 45.00;
};


var myMacbookPro = new MacbookPro();

// Outputs: 900.00
console.log( myMacbookPro.getPrice() );


// Decorate the macbook
var decoratedMacbookPro = new CaseDecorator( myMacbookPro );

// This will return 945.00
console.log( decoratedMacbookPro.getPrice() );

Looks good to me on first sight. However lately I was analysing this code a bit deeper and ran into some issues that I want to share with you today. Let's start: The current version of this code contains this piece:

MacbookDecorator.prototype = {
    addEngraving: function(){
        return this.macbook.addEngraving();
    },
    addParallels: function(){
        return this.macbook.addParallels();
    },
    add4GBRam: function(){
        return this.macbook.add4GBRam();
    },
    add8GBRam:function(){
        return this.macbook.add8GBRam();
    },
    addCase: function(){
        return this.macbook.addCase();
    },
    getPrice: function(){
        return this.macbook.getPrice();
    }
};

Which at this point it doesn't make sense actually (catch me if I'm wrong), especially when beeing called in the given ways using:

var myMacbookPro = new MacbookPro();

// Outputs: 900.00
console.log( myMacbookPro.getPrice() );


// Decorate the macbook
var decoratedMacbookPro = new CaseDecorator( myMacbookPro );

// This will return 945.00
console.log( decoratedMacbookPro.getPrice() );

There's no way the MacbookDecorator.prototype makes sense in the given context. Some of you might say it's part of the chain thus the protype of MacbookDecorator has its' place there. But no. It's at no place called in a way here that it is of practical use. Why?

extend( CaseDecorator, MacbookDecorator );

This, may some of you believe, be the part where the magic happens, but baring this piece in mind:

function extend( a, b ){
    for( var key in b )
        if( b.hasOwnProperty(key) )
            a[key] = b[key];
    return a;
}

There is no inheritance taking place to the CaseDecorator from the MacbookDecorator, in fact, the prototype in that way is ignored completely and not beeing inherited by CaseDecorator. Additionally, the extend function has no use at all, since it'll only read properties from objects, however both of the arguments supplied are functions, thus, no inheritance will take play if the functions are not at least executed before beeing handed, which is tricky, since this.macbook is not even defined at that point.

Here is my version of the code, practically doing exactly the same thing, leaving out the uneccesary and partly quite irritating parts:

// Constructor.
var Interface = function (name, methods) {
        if (arguments.length != 2) {
            throw new Error("Interface constructor called with " + arguments.length + "arguments, but expected exactly 2.");
        }
        this.name = name;
        this.methods = [];
        for (var i = 0, len = methods.length; i < len; i++) {
            if (typeof methods[i] !== 'string') {
                throw new Error("Interface constructor expects method names to be " + "passed in as a string.");
            }
            this.methods.push(methods[i]);
        }
    };


// Static class method.
Interface.ensureImplements = function (object) {
    if (arguments.length < 2) {
        throw new Error("Function Interface.ensureImplements called with " + arguments.length + "arguments, but expected at least 2.");
    }
    for (var i = 1, len = arguments.length; i < len; i++) {
        var interface = arguments[i];
        if (interface.constructor !== Interface) {
            throw new Error("Function Interface.ensureImplements expects arguments" + "two and above to be instances of Interface.");
        }
        for (var j = 0, methodsLen = interface.methods.length; j < methodsLen; j++) {
            var method = interface.methods[j];
            if (!object[method] || typeof object[method] !== 'function') {
                throw new Error("Function Interface.ensureImplements: object " + "does not implement the " + interface.name + " interface. Method " + method + " was not found.");
            }
        }
    }
};




var Macbook = new Interface( "Macbook",
  ["addEngraving",
  "addParallels",
  "add4GBRam",
  "add8GBRam",
  "addCase"]); /* Gives back an object that stores the name of the Interface and the array of methods (pushed) */

// A Macbook Pro might thus be represented as follows:
var MacbookPro = function(){};

var decorator = function() {
  return {
    addEngraving: function(){
    },
    addParallels: function(){
    },
    add4GBRam: function(){
    },
    add8GBRam:function(){
    },
    addCase: function(){
    },
    getPrice: function(){
      // Base price
      return 900.00;
    }
  }
} 
MacbookPro.prototype = decorator();
//var MacbookDecorator = function( macbook ){
//    Interface.ensureImplements( macbook, Macbook );
//    this.macbook = macbook;
//}
/*MacbookDecorator.prototype = decorator();*/


var CaseDecorator = function( macbook ){
   this.macbook = macbook;
};


CaseDecorator.prototype.addCase = function(){
    return this.macbook.addCase() + "Adding case to macbook";
};

CaseDecorator.prototype.getPrice = function(){
    return this.macbook.getPrice() + 45.00;
};


// Decorate the macbook
var decoratedMacbookPro = new CaseDecorator( new MacbookPro() );

// This will return 945.00
console.log( decoratedMacbookPro.getPrice() );

What do you think about this, does the author miss something, or is it true?

Upvotes: 4

Views: 212

Answers (1)

Bergi
Bergi

Reputation: 665584

extend( CaseDecorator, MacbookDecorator );

There is no inheritance taking place to the CaseDecorator from the MacbookDecorator, in fact, the prototype in that way is ignored completely and not beeing inherited by CaseDecorator.

You're completely right about this. It looks like a lapse of the author to me, and should actually have been

extend(CaseDecorator.prototype, MacbookDecorator.prototype);

or even a complete prototypal inheritance setup.

extend will only read properties from objects, however both of the arguments supplied are functions

Don't forget that functions are objects as well, so extend on them could do a lot on them if they only had own properties. But you're right, in this case it does nothing, as typical function instance properties like .prototype or .name are non-enumerable.

Upvotes: 1

Related Questions