Aren't decorators easily breaking the ISP?

Question

"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?

Answer 1

//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.

Answer 2

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.