Interface Segregation Principle – Resolving Contradicting Definitions

design-principlessolid

When reading articles on ISP, there seem to be two contradicting definitions of ISP:

According to first definition ( see 1, 2, 3), ISP states that classes implementing the interface shouldn't be forced to implement functionalities which they don't need. Thus, fat interface IFat

interface IFat
{
     void A();
     void B();
     void C();
     void D();
}

class MyClass: IFat
{ ... }

should be split into smaller interfaces ISmall_1 and ISmall_2

interface ISmall_1
{
     void A();
     void B();
}

interface ISmall_2
{
     void C();
     void D();
}

class MyClass:ISmall_2
{ ... }

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() and B():

But according to the second definition ( see 1 , 2, answer by Nazar Merza), ISP states that MyClient calling methods on MyService shouldn't be aware of methods on MyService that it doesn't need. In other words, if MyClient only needs the functionality of C() and D(), then instead of

class MyService 
{
    public void A();
    public void B();
    public void C();
    public void D();
}

/*client code*/      
MyService service = ...;
service.C(); 
service.D();

we should segregate MyService's methods into client-specific interfaces:

public interface ISmall_1
{
     void A();
     void B();
}

public interface ISmall_2
{
     void C();
     void D();
}

class MyService:ISmall_1, ISmall_2 
{ ... }

/*client code*/
ISmall_2 service = ...;
service.C(); 
service.D();

Thus with the former definition, the goal of ISP is to "make the life of classes implementing IFat interface easier", while with the latter the goal of ISP is to "make the life of clients calling methods of MyService easier".

Which of the two different definitions of ISP is actually correct?

@MARJAN VENEMA

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

While it makes sense to put cohesive methods within the same interface, I thought with ISP pattern the needs of the client take precedence over the "cohesiveness" of an interface. In other words, I thought with ISP we should lump within the same interface those methods needed by particular clients, even if that means leaving out of that interface those methods that should, for the sake of cohesiveness, also be put inside that same interface?

Thus, if there were lots of clients that will only ever needed to call CutGreens, but not also GrillMeat, then to adhere to ISP pattern we should only put CutGreens inside ICook, but not also GrillMeat, even though the two methods are highly cohesive?!

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.

By "implementing classes not following SRP" are you referring to those classes that implement IFat or to classes that implement ISmall_1 / ISmall_2? I assume you're referring to classes that implement IFat? If so, why do you assume they don't already follow SRP?

thanks

Best Answer

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.

Related Solutions

Do Decorators Violate the ISP in Object-Oriented Design?

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

Object-oriented – Does interface segregation principle apply to concrete methods

In your example, CarShop does not depend on isQualityPass, and it is not forced to make an empty implementation for a method. There is not even an interface involved. So the term "ISP" simply does not match here. And as long as a method like isQualityPass is a method which fits well into the Car object, without overburden it with additional responsibilities or dependencies, this is fine. There is no need to refactor a public method of a class into another place just because there exists one client not using the method.

However, making a domain class like Car directly dependend on something like HttpClient is probably not a good idea either, irrespectively which clients use or not use the method. Moving the logic into a separate class CheckCarQualityPass is just not called "ISP", this is called "separation of concerns". The concern of a reusable car object should probably not be to make any external HTTP calls, at least not directly, this limits the reusability and moreover the testability too much.

If isQualityPass cannot be easily moved out to another class, the alternative would be to make the Http calls through an abstract interface IHttpClient which is injected into Car at construction time, or by injecting the whole "QualityPass" checking strategy (with the Http request encapsulated) into the Car object. But that is IMHO only the second best solution, since it increases the overall complexity instead of reducing it.