search

addyosmani / essential-js-design-patterns

Repo for my 'Learning JavaScript Design Patterns' book
http://addyosmani.com/resources/essentialjsdesignpatterns/book/
4.82k stars 792 forks source link

Decorator Pattern extend() #222

Open iramirezc opened 5 years ago

iramirezc commented 5 years ago

Hi Addy,

I wonder why in this line of code https://github.com/addyosmani/essential-js-design-patterns/blob/21346c134aa953ba469782372c90e42c36a1f083/book/snippets/01-JavaScript-Design-Patterns/12-the-decoration-pattern.es5.js#L255 we are extending CaseDecorator with MacbookDecorator. As functions, they don't have any enumerable properties by their own yet. Shouldn't happen the extension in the prototype like:

extend(CaseDecorator.prototype, MacbookDecorator.prototype);

because trying to do this console.log(decoratedMacbookPro.addEngraving()); results in a TypeError: decoratedMacbookPro.addEngraving is not a function

iramirezc commented 5 years ago

Also, in this constructor: https://github.com/addyosmani/essential-js-design-patterns/blob/21346c134aa953ba469782372c90e42c36a1f083/book/snippets/01-JavaScript-Design-Patterns/12-the-decoration-pattern.es5.js#L249

shouldn't CaseDecorator be a "subclass" of MacbookDecorator as we learned from the Mixins Pattern:

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

That way, when doing https://github.com/addyosmani/essential-js-design-patterns/blob/21346c134aa953ba469782372c90e42c36a1f083/book/snippets/01-JavaScript-Design-Patterns/12-the-decoration-pattern.es5.js#L274 we are ensuring that the MacbookPro instance fulfills the required interface.