Refactoring of a client calling a Rest Service

Question

I have a class that is responsible for calling a service and returning a response.

The contract is like this

interface ServiceClient {
    ServiceResponse process(ServiceSubmissionParams params);
}

ServiceSubmissionParams and ServiceResponse are simple POJOs that are part of the application calling the service (and not part of Service API).

ServiceSubmissionParams has a bunch of data members that are needed to call an API of the service and ServiceResponse has data that is interpreted from the service response.

The implementation class is like this

class ServiceClientImpl implements ServiceClient {
     private Client client; //JAXRS client

     ServiceResponse process(ServiceSubmissionParams params) {
         //1
         HttpHeaders headers = createHeaders(params);
         //2
         Response response = //call service with the above headers
         //3 - Build reponse
         if (response.getStatusInfo().getFamily() == Response.Status.Family.SUCCESSFUL) {
           // Build ServiceResponse
         }
         //4 
         throw handleErrorResponse(response);
    }
    HttpHeaders createHeaders(ServiceSubmissionParams params) {
      //Builds HttpHeaders from ServiceSubmissionParams fields
    } 

    RuntimeException handleACAPIErrorResponse(Response response) {
     /*
        Interprets the response to throw a (custom) subclass of RuntimeException
      if response == 503, return ServiceUnavailableException
      if response == 408, return RequestTimeoutException
      .... 
      if response == 500, return ServerException

     else return UnknownException
     */
    }

}

The reason for throwing specific (or different) RuntimeExceptions is that based on the type some gets retried (ServiceUnavailable or RequestTimeout) while others don't.

My question:

I feel that this class does a lot of work. Prepare headers, call service, interpret response. So, does it violate SRP? Or does it do only job considering the level of abstraction of taking a ServiceSubmissionParams and returning a ServiceResponse.

More than a violation of SRP, I need to change this class whenever I need to handle an error response from the service i.e, I need to read the response body in case of 500 or 400 response code and interpret it to populate a ServiceResponse. So, should I consider splitting this class into multiple classes?

Answer 1

There are usually different ways to look at the responsibilities a class has, in the context of SRP. If you consider that its responsibility is "encapsulate REST service calls, so that clients of this class do not have to worry about HTTP & co.", which is OK in my book even if rather broad, then having a single class is no big deal -- provided this is not a huge API resulting in a client that is hundreds if not thousands of lines long.

Remember that SRP is all about coupling reduction, but code should remain cohesive, i.e. related elements should be close to one another.

If the class is so big that working with it is difficult, then it makes sense to look into splitting it. In that case, the responsibility described above becomes that of the module (I talk here of a conceptual module, not a Java 9 one) the classes will be part of, with each class inside having a more precise responsibility.

With the problem you have of having to manage many different responses, one way to do it would have to define a ResponseHandler interface with implementations that each manage a single case, and have your client implementation class be configured to use a given ResponseHandler depending on the response status (ideally configured in a Map<HttpStatus, ResponseHandler> so that you do not have to manage a chain of ifs in the code, or in a List|Set<ResponseHandler> with each handler defining a canHandle(Response) method if determining which one to use is not as simple as just checking the status code). That way managing a new case would be as simple as adding a new ResponseHandler implementation and updating the list of objects used by ServiceClientImpl.