"The Interface-Segregation Principle (ISP) states that no client should be forced to depend on methods it does not use."

The decorator pattern is a design pattern to decorate a method of a class. For this, the decorator implements the same interface as the decorated object, so it can take its place.

But doesn't this mean that it has to implement all of the methods in the interface, when it uses only one of them, the one it decorates? So I guess it violates the ISP. The only case it doesn't it is when the decorated interface only contains the method that needs to be decorated, but I see it as a rather rare case.

An example would be that there are pages in a book. Some pages have footnotes, other pages have titles at the top. So we implement these extras as decorators of a method, let's say draw(). But a page has other methods as well, let's say: fold() and tear() and other methods can come in later. But if we want to use decorated versions of pages inserted in a book, and working the same way, we end up implementing in every decorator every method of a page, where the only thing happening is that the decorator passes the call to the page it's containing.

interface IPage {
    public ITearResult tear();
    public void draw();
}

class FooterDecorator implements IPage {
    private IPage page;

    public FooterDecorator(IPage page) {
        this.page = page; //the object it decorates
    }

    //violates the ISP, because it has to implement it, but it's not using it,
    //just passes the operation
    public ITearResult tear() {
        return page.tear();
    }

    //decorated method
    public void draw() {
        page.draw();
        ... draw footer - the decoration
    }
}

Am I not understanding the decorator pattern or the ISP correctly? Is it an acceptable violation or do these two really contradict each other?

  • In this case, the decorator is not the client of the decorated class, it is an augmentation. As such, it is inside the tent. – BobDalgleish Apr 23 '14 at 19:46
  • 3
    I would say that it's not 100% technically accurate to say that the decorator doesn't need the additional methods, since it uses them to delegate to the decorated object. – Aviv Cohn Apr 23 '14 at 19:47
  • 1
    In some languages you can add one more layer of abstraction between FooterDecorator and IPage say PageBaseDecorator that simply implements all of the boiler-plate for the interface and delegates all of the functionality to the page member, then FooterDecorator would be a child of PageBaseDecorator and would only need to override the functionality that it handles differently. – YoungJohn Apr 28 '14 at 21:47
up vote 11 down vote accepted
//violates the ISP, because it has to implement it, but it's not using it,
//just passes the operation
public ITearResult tear() {
    return page.tear();
}

This is not correct. The decorator is using the tear method; it's implemented it according to its contract, even if the implementation is as simple as delegating that job to someone else. From the point of view that the decorator is the client of the original interface, there's no problem; the whole point of the decorator is to implement the original interface and more, so by definition it has a dependency on the interface.

The user of a decorator also has a dependency on the original interface and whatever augmentation the decorator provides; otherwise, he wouldn't need a decorator and could instead rely on an interface that provides only the new features.

Am I not understanding the decorator pattern or the ISP correctly, it's an acceptable violation or these two really contradict each other?

You're not understanding ISP correctly.

Consider a system with various types of doors. Doors implement an interface Door that contains methods open and close. The client code calls these methods and all is well.

Later on you need to implement a TimedDoor that closes on its own after a certain amount of time (but can still be closed through the close method before the time elapses.) Some client code out there needs to be able to set the timeout on these doors. What the Interface Segregation Principle tell us is that you shouldn't go add a setTimeout method to the Door interface; instead, we should define a new interface (say, TimedObject) that provides those facilities.

Consider what would happen if you added setTimeout to Door - now every door in the system has to implement that method. What should they do? Treat it as a no-op? Throw an exception? Both are violations of the setTimeout method's contract (i.e violations of Liskov's Subtitution Principle.) The only other option is to weaken the contract by saying that objects may or may not implement setTimeout, which is tantamount to saying that something may or may not be a TimedObject. You'd be fighting the type system and robbing yourself of the guarantees it's meant to buy you.

Another (much less important but still unfortunate) side effect is that modifying the Door interface would generally trigger a recompile of all code that relied on it even though that code has no use for the setTimeout method.

With that in mind, note that using a decorator doesn't modify the original interface it wraps.

  • Thank you, I think I understand now. My biggest problem was that whenever I added a functionality to the decorated object I also needed to delegate the operation in every decorator. Thus I thought it is anti pattern, but you are right in the sense that the implementation is defined, even if it's as simple as delegation. – Máthé Endre-Botond Apr 23 '14 at 20:21
  • 1
    @SinistraD Delegation does require a fair bit of boilerplate, which is an unfortunate consequence of functions/methods not being first-class things in Java/C# (wouldn't it be nice to say val tear = page.tear?). Even so, delegation is a good thing - you should favor composition over inheritance. You can always compose multiple objects, but you can rarely inherit from multiple classes, and when you can, it's never pretty. – Doval Apr 23 '14 at 20:24
  • 1
    example stolen from object mentor ;-) – Lovis Apr 28 '14 at 13:39
  • @DonL Thanks, I couldn't remember if I'd read it in one of Bob Martin's freely-available articles or in Clean Code. – Doval Apr 28 '14 at 13:44

You may be missing a layer of abstraction in your decorator implementation. Consider the following alternative:

interface IPage {
public ITearResult tear();
public void draw();
}

abstract class PageBaseDecorator implements IPage {
    protected IPage page;

    public PageBaseDecorator(IPage page) {
        this.page = page; //the object it decorates
    }

    public ITearResult tear() {
        return page.tear(); // delegate implementation to object
    }

    public void draw() {
        page.draw(); // delegate implementation to object
    }
}

class FooterDecorator extends PageBaseDecorator {

    public FooterDecorator(IPage page) {
        super(page);
    }

    // tear does not need to be implemented as it exists in PageBaseDecorator
    // and is not used by FooterDecorator

    //decorated method
    public void draw() {
        super.draw();
        ... draw footer - the decoration
    }
}

I'm not up on my Java programming so I'm not certain if the syntax above is correct, but it should give you an idea of what you might be missing in the decorator pattern. The Base Decorator is not violating ISP because it depends on the entire interface it is delegating to. The children of the Base Decorator should only need to implement the pieces they actively decorate though.

  • 1
    I used this solution a lot since I asked the question, works great most of the time. Thanks. – Máthé Endre-Botond Jul 7 '15 at 6:22

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.