10

As interface segregation principle suggests no client should be forced to depend on methods it doesn't use, so a client should not implement a empty method for its interface methods, otherwise this interface method should be put into another interface.

But how about concrete methods? Should I separate the methods that not every client would use? Consider the following class:

public class Car{
    ....

    public boolean isQualityPass(){
        ...
    }

    public int getTax(){
        ...
    }

    public int getCost(){
        ...
    }
}

public class CarShop{
    ...
    public int getCarPrice(int carId){
        Car car=carList[carId];
        int price=car.getTax() + car.getCost()... (some formula);
        return price;
    }
}

at above code, CarShop doesn't use method isQualityPass() in Car at all, should I separate isQualityPass() into a new class:

public class CheckCarQualityPass{
    public boolean isQualityPass(Car car){
    }
}

in order to reduce coupling of CarShop? Because I think once if isQualityPass() needs extra dependency, e.g.:

public boolean isQualityPass(){
    HttpClient client=...
}

CarShop would depend on HttpClient even it never uses HttpClient actually. So my question is: according to interface-segregation principle, should I separate concrete methods that not every clients would use, so that those methods depend on client only when the client actually uses, in order to reduce coupling?

  • 2
    Does usually a car know when it passes "quality"? Or it's maybe a business rule that could be encapsulated by its own? – Laiv Jun 12 '17 at 8:15
  • 2
    As the word interface in ISP implies it's about interfaces. So if you have a method in your Car class that you don't want (all) users to know about then create (more than one) interface that the Car class implements an which declare only methods useful in the interfaces context. – Timothy Truckle Jun 12 '17 at 8:20
  • @Laiv I'm sure we're going to see vehicles which know much more than that pretty soon. ;) – unified modeling sandwich Jun 12 '17 at 8:26
  • 1
    A car will know what its maker wants him to know. Volkswagen know what I'm talking about :-) – Laiv Jun 12 '17 at 8:42
  • 1
    You mention an interface, but there is no interface in your example. Are we talking about turning Car into an interface and which methods to include in said interface? – Neil Jun 12 '17 at 10:29
6

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.

  • what about Strategy pattern to resolve the isQualityPass method? – Laiv Jun 12 '17 at 12:46
  • @Laiv: technically, this will work, sure (see my edit), but it will result in a more complex Car object. It would not be my first choice for a solution (at least not in context of this contrived example). However, it may make more sense in the "real" code, I don't know. – Doc Brown Jun 12 '17 at 13:01
6

So my question is: according to interface-segregation principle, should I separate concrete methods that not every clients would use, so that those methods depend on client only when the client actually uses, in order to reduce coupling?

The interface segregation principle is not about disallowing access to what you don't need. It's about not insisting on access to what you don't need.

Interfaces are not owned by the class that implements them. They're owned by the objects that use them.

public class CarShop{
    ...
    public int getCarPrice(int carId){
        Car car=carList[carId];
        int price=car.getTax() + car.getCost()... (some formula);
        return price;
    }
}

What's used here is getTax() and getCost(). What's being insisted upon is everything accessible through Car. The problem is insisting on Car means it's insisting on access to isQualityPass() which isn't needed.

This can be fixed. You ask if it can be fixed concretely. It can.

public class CarShop{
    ...
    public int getCarPrice(int carId){
        CarLiability carLiability=carLiabilityList[carId];
        int price=carLiability.getTax() + carLiability.getCost()... (some formula);
        return price;
    }
}

None of that code even knows if CarLiability is an interface or a concrete class. That's a good thing. It doesn't want to know.

If it's an interface Car might implement it. This wouldn't violate ISP because even though isQuality() is in Car CarShop doesn't insist on it. This is fine.

If it's concrete it might be that isQuality() either doesn't exist or has been move off to somewhere else. This is fine.

It also might be that CarLiability is a concrete wrapper around Car that is delegating work to it. So long as CarLiability doesn't expose isQuality() then CarShop is fine. Of course this just kicks the can down the road and CarLiabilityhas to figure out how to follow ISP with Car the same way CarShop had to do.

In short, isQuality() doesn't need to be removed from Car because of ISP. The implied need for isQuality() needs to be removed from CarShop because CarShop doesn't need it, so it shouldn't ask for it.

4

Does interface segregation principle apply to concrete methods?

But how about concrete methods? Should I separate the methods that not every client would use?

Not really. There are different ways to hide Car.isQualityPass from CarShop.

1. Access modifiers

From the Demeter's Law point of view, we could consider Car and CardShop not to be friends. It legitimates us to do the next.

package com.my.package.domain.model
public class Car{
    ...
    protected boolean isQualityPass(){...}
}

package com.my.package.domain.services
public class CarShop{
    ...
}

Be aware of both components are in different packages. Now CarShop has no visibility over Car protected behaviours. (Excuse me in advance if the example above looks so simplistic).

2. Interface Segregation

The ISP works from the premise that we work with abstractions, not with concrete classes. I will assume that you are already familiar with the ISP implementation and with the role interfaces.

Despite the actual Car implementation, nothing stop us from practising ISP.

//role interfaces 
public interface Billable{
   public int getCosts();
   public int getTaxs();
}

//role interfaces
public interface QualityAssurance{
   public boolean isQualityPass();
}

public class Car implements Billable, QualityAssurance{
   ...
}

public class CarShop {
  ...
  public int getPrice(Billable billable){
     return billable.getCosts() * billable.getTaxs();
  }
}

What I have done here. I have narrowed down the interaction between Car and CarShop through the role interface Billable. Be aware of the change on the getPrice signature. I have modified intentionally the argument. I wanted to make obvious that CarShop is only "tied/bound" to one of the role interfaces available. I could have followed the actual implementation but I don't know the real implementation details and I'm afraid of the actual getPrice(String carId) has access (visibility) over the concrete class. If it has, all the work done with the ISP becomes useless because it's in the developer's hands to do casting and work only with the interface Billable. No matter how methodic we are, the temptation is going to be always there.

3. Single responsibility

I'm afraid I'm not in a position to say if the dependency between Car and HttpClient is adequate, but I agree with @DocBrown, it raises some warnings that worthwhile a design review. Neither Demeter's Law nor ISP will make your design "better" at this point. They will just mask the issue, not fix it.

I have suggested DocBrown the Strategy Pattern as a possible solution. I agreed with him that the pattern adds complexity, but I also think that any re-design will. It's a trade-off, the more decoupling we want, the more moving parts we have (usually). Anyways, I think both agree with a re-design is highly advisable.

Summing up

No, you don't need to move concrete methods to outer classes for don't make them accessible. There could be countless consumers. Would you move all the concrete methods to outer classes every time a new consumer comes into a play? I hope you don't.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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