Applying Interface Segregation Principle to service layer interface-CodePudding

Using the book as analogy, suppose I have a book resource with the following API (there are also update and delete etc but for simplicity only show two here)

GET /book/{id}
POST /book

Each of these API would call other APIs to get the result, rather than the typical CRUD database operation. And based on the requirement / existing framework constraint, there are separate two controller class GetBookController and CreateBookController. The controller is to handle the request and response. So the actual business logic and retrieve / create book are in the service layer.

The question then is, should there be a separate interface for each of book operation(GetBookService and CreateBookService), or to have just only one (BookService)?

Based on the Interface Segregation Principle which states "Clients should not be forced to depend upon interfaces that they do not use". Here the GetBookController class is the client, where it only has to query book without creating it, so it only requires GetBookService. If it is to use BookService, it doesn't use the method createBook, which seems to violate ISP principle. However, if using separate interface, it would result in many interface and implementation classes being created. Am I misunderstanding the ISP principle?

@Controller
public class GetBookController {
    
    @Autowired
    private GetBookService getBookService;
}

@Controller
public class CreateBookController {
    
    @Autowired
    private CreateBookService createBookService;
}

public interface GetBookService {
    Book getBook(long id);
}

public interface CreateBookService {
    Boolean createBook(Book request);
}

public interface BookService {
    Book getBook(long id);
    Boolean createBook(Book request);
}

CodePudding user response：

So, what I usually do for a CRUD entity like a book is a single BookControler. You do not need multiple book controllers, as it is a Controller - it controls things associated with books. So it should be one class.

Again the same logic for the service. One book service interface is sufficient. It deals with the needs of the books and returns the result of a request.

So, yes, if you make an interface for each method, there would be too many interfaces, which is not easy to deal with as the project scales. Also, it makes each interface do very little (which in some cases is desired, but not this one).

A good analogy for the ISP principle would be: You do not want to make a coffee machine need to implement how to print a paper, and a printer to implement how to make coffee. But a coffee machine making different coffees is in its scope.

 @Controller
 public class BookController {
     private final BookService bookService;

     @Autowired
      private BookController (BookService bookService) {
          this.bookService = bookService;
      }
  }
    
  public interface BookService {

       Book getBook(long id);
    
       Boolean createBook(Book request);
  }

And for example, you can use autowiring from the constructor, for easier mocking.

CodePudding user response：

the ISP principle goal is to reduce the side effects and frequency of required changes by splitting the software into multiple, independent parts.

but the question is how small enough is required to split the software component .

In your case you are over applying the principle , for example suppose you have class :

class Student {
String name;
Integer age;
ClassRoom classRoom;

.
.
.
setters ....
getters ....
}

and one dev think it better to split it up so he ended up with making separate classes for each setters and getters (which is clearly unnecessary )

the same here with your case it's better to have one BookController and one BookService that covers all required operations (create , get , update , delete).

unless you have a different type of BookService ( for example ElectronicBook , PaperBook) then you can have different implementation of BookService interface.

CodePudding user response：

You should not be designing the BookService interface(s) as if you know about all clients that it might have. Usually there should be only one BookService interface, the purpose of which is to provide CRUD access to books.

One exception to this rule would be if the book service supported different client roles with different access levels. If this case you might have a BookReaderService that provides only R, and a BookUpdaterService that extends it and provides full CRUD.

Now, you might want to write your clients so that they didn't depend on operations that they don't need (ISP). In this case, you would define, on the client side, the interfaces that the clients want, and then use an adapter to provide those interfaces based on the appropriate book service... But really that is overkill in this case. Your controllers are already adapters, so adding another adapter layer is redundant.

CodePudding user response：

it would result in many interface and implementation classes being created

Yeah, you are right

The question then is, should there be a separate interface for each of book operation(GetBookService and CreateBookService), or to have just only one (BookService)?

ISP is how interface can be consumed. So, whether it is necessary to consume other methods of interface or don’t depend on more than you need.

An example with HDD that uses ISP:

public interface IReadable
{
    string Read();
}

public interface IWriteable
{
    void Write();
}

public class HDD : IReadable, IWriteable
{
    public string Read() { }

    public void Write()  { }
}

By creating one interface for Read() and Write() methods, it would obligate class to implement both methods in class. But some classes only want to read data, others want to write data, and some to do both. E.g. card reader might want to read data. So in this case it is better to create separate interfaces.

So let's look another example with CardReader. CardReader just reads data, it does not write data. So, if we inherit one interface with Read() and Write() methods, then we would violate ISP principle. An example of violation of ISP:

public interface IWriteReadable
{
    string Read();
    void Write();
}

public class CardReader : IWriteReadable
{
    // this is a necessary method
    public string Read() { }

    // here we are obligating to implement unnecessary method of interface
    public void Write() { }
}

So by applying ISP, you only puts methods in interface that are necessary for the client class. If your class/client just wants to read data, then you need to use IReadable interface, not IReadableWriteable.