HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Synchronous / asynchronous REST client

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
asynchronousrestclientsynchronous

Problem

I have working code with original design, and now I had a slight design change so trying to code review that as well. I already had code review on my original design here.

Original Design:

I am using RestTemplate as my HttpClient to execute a URL and then my server will return a JSON string as the response. The customer will call this library by passing DataKey object which has userId in it.

  • Using the given userId, I will find out what are the machines that I can hit to get the data and then store those machines in a ArrayList, so that I can execute them sequentially.



  • After that I will check whether the first hostname is in block list or not. If it is not there in the block list, then I will make a URL with the first hostname in the list and execute it and if the response is successful then return the response. But let's say if that first hostname is in the block list, then I will try to get the second hostname in the list and make the url and execute it, so basically, first find the hostname which is not in block list before making the URL.



  • Now, let's say if we selected first hostname which was not in the block list and executed the URL and somehow server was down or not responding, then I will execute the second hostname in the list and keep doing this until you get a successful response. But make sure they were not in the block list as well so we need to follow above point.



  • If all servers are down or in block list, then I can simply log and return the error that service is unavailable.



I am making a library in which I will have synchronous (getSyncData) and asynchronous (getAsyncData) methods in it.

  • getSyncData() - waits until I have a result, returns the result.



  • getAsyncData() - returns a Future immediately which can be processed after other things are done, if needed.



Below is my DataClient class which will be called by customer and they will pass DataKey object to either getSyncData or getAsyncData method de

Solution

In call(), you have duplicate code:

} catch (HttpClientErrorException ex) {
            HttpStatusCodeException httpException = ex;
            DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
            String errorMessage = httpException.getResponseBodyAsString();
            return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            // logging exception here                   
        } catch (HttpServerErrorException ex) {
            HttpStatusCodeException httpException = ex;
            DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
            String errorMessage = httpException.getResponseBodyAsString();
            return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            // logging exception here                                       
        }


Since Java 7, you can remove the duplicate code with a multiple-catch:

} catch (HttpClientErrorException | HttpServerErrorException ex) {
            HttpStatusCodeException httpException = ex;
            DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
            String errorMessage = httpException.getResponseBodyAsString();
            return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            // logging exception here                                       
        }


In the design change, it is in performDataRequest().

In the call() method of the design change, you have this line:

List> responseFutureList = new ArrayList>();


If the code that is calling the method performs minimal gets to the list, it is preferred to use LinkedList instead, as LinkedList is faster at adding but slower at getting.

Same thing here:

List keys = new ArrayList<>();


I don't know much about this topic; I'll leave that to the more experienced programmers.

Code Snippets

} catch (HttpClientErrorException ex) {
            HttpStatusCodeException httpException = ex;
            DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
            String errorMessage = httpException.getResponseBodyAsString();
            return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            // logging exception here                   
        } catch (HttpServerErrorException ex) {
            HttpStatusCodeException httpException = ex;
            DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
            String errorMessage = httpException.getResponseBodyAsString();
            return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            // logging exception here                                       
        }
} catch (HttpClientErrorException | HttpServerErrorException ex) {
            HttpStatusCodeException httpException = ex;
            DataErrorEnum error = DataErrorEnum.getErrorEnumByException(httpException);
            String errorMessage = httpException.getResponseBodyAsString();
            return new DataResponse(errorMessage, error, DataStatusEnum.ERROR);
            // logging exception here                                       
        }
List<Future<DataResponse>> responseFutureList = new ArrayList<Future<DataResponse>>();
List<DataKey> keys = new ArrayList<>();

Context

StackExchange Code Review Q#113728, answer score: 7

Revisions (0)

No revisions yet.