Do Decorators Violate the ISP in Object-Oriented Design?

designobject-orientedsolid

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

Best Answer

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

Both are correct

The way I read it, the purpose of ISP (Interface Segregation Principle) is to keep interfaces small and focused: all interface members should have very high cohesion. Both definitions are intended to avoid "jack-of-all-trades-master-of-none" interfaces.

Interface segregation and SRP (Single Responsibility Principle) have the same goal: ensuring small, highly cohesive software components. They complement each other. Interface segregation ensures that interfaces are small, focused and highly cohesive. Following the single responsibility principle ensures that classes are small, focused and highly cohesive.

The first definition you mention focuses on implementers, the second on clients. Which, contrary to @user61852, I take to be the users/callers of the interface, not the implementers.

I think that your confusion stems from the a hidden assumption in the first definition: that the implementing classes are already following the single responsibility principle.

To me the second definition, with the clients as the callers of the interface, is a better way of getting to the intended goal.

Segregating

In your question you state:

since this way my MyClass is able to implement only the methods it needs ( D() and C() ), without being forced to also provide dummy implementations for A(), B() and C():

But that is turning the world upside down.

A class implementing an interface does not dictate what it needs in the interface it is implementing.
The interfaces dictate what methods an implementing class should provide.
The callers of an interface really are the ones that dictate what functionality they need the interface to provide for them and thus what an implementer should provide.

So when you are going to split IFat into smaller interface, which methods end up in which ISmall interface should be decided based on how cohesive the members are.

Consider this interface:

interface IEverythingButTheKitchenSink
{
     void DoDishes();
     void CleanSink();
     void CutGreens();
     void GrillMeat();
}

Which methods would you put in ICook and why? Would you put CleanSink together with GrillMeat just because you happen to have a class that does just that and a couple of other things but nothing like any of the other methods? Or would you split it into two more cohesive interfaces, such as:

interface IClean
{
     void DoDishes();
     void CleanSink();
}

interface ICook
{
     void CutGreens();
     void GrillMeat();
}

Interface declaration note

An interface definition should preferably be on its own in a separate unit, but if it absolutely needs to live with either caller or implementer, it should really be with the caller. Otherwise the caller gets an immediate dependency on the implementer which is defeating the purpose of interfaces altogether. See also: Declaring interface in the same file as the base class, is it a good practice? on Programmers and Why should we place interfaces with classes that use them rather than those that implement them? on StackOverflow.

Object-oriented – Using the Decorator pattern to add public methods to an object

The decorator pattern is typically used to avoid an explosion of subclasses. A common example involves UI windows that you may want to have any combination of n attributes (scrollbar, titlebar, resizable, movable, etc.).
Supporting all combinations of all those N attributes for all UI windows M subclasses would involve making N * M subclasses.

The decorator pattern prevents the explosion by wrapping around the base window class' existing public methods.
The decorator pattern ideally would only require N + M classes in this case.

For the case you have described, you are adding new public methods; therefore, the decorator pattern probably is not ideal. I cannot think of any situations where it would be ideal to add new methods via a decorator.

A few alternatives to consider:

Subtyping

It is valid to use subtyping to add methods, provided you are keeping the SOLID principles in mind. Therefore, you might consider extending the concrete object to support methodB and then design the decorator around it.

AbstractObject object = new ExtendedConcreteObject();
object.methodA(); // does stuff
object = new ConcreteDecorator(object);
(ConcreteDecorator)object.methodB();
// methodB() belongs to ExtendedConcreteObject() but not to AbstractObject()

Adapter Pattern

Another way to view the problem: by adding a public method, you are changing the object's interface. This can be done by using the adapter pattern. To keep the concrete decorator simple, you might consider creating a decorator and then building an adapter for that decorator.

AbstractObject object = new ConcreteObject();
object.methodA(); // does stuff
object = new ConcreteDecorator(object);
object = new ConcreteAdapter(object);
(ConcreteAdapter)object.methodB();
// methodB() belongs to ConcreteAdapter() but not to AbstractObject() or ConcreteDecorator().