Get rid of response.getStatusCode() / 100 == 2
. Write response.getStatusCode() == 200
or response.getStatusCode() == HttpStatus.SC_OK
instead.
Remove the else
branch and throw
after the if
statement.
Rename the method to getExceptionByStatusCode
or generateExceptionForStatusCode
. You don't handleException
, you decide which one to throw.
Choose the right return type. Don't use RuntimeException
. It can be ResponseStatusException
or any other type appropriate to the HTTP/your domain abstraction.
For each case, decide what type you want to return. Again, not RuntimeException
.
A bit improved version would be
private String handleResponse(HttpResponse response) {
final int statusCode = response.getStatusCode();
if (statusCode == HttpStatus.SC_OK) {
return response.getEntity().toString();
}
throw getExceptionByStatusCode(statusCode);
}
private MyDomainHTTPException getExceptionByStatusCode(int statusCode) {
switch (statusCode) {
case HttpStatus.SC_NOT_FOUND:
return new MyDomainHTTPException("...");
case HttpStatus.SC_UNAUTHORIZED:
return new MyDomainHTTPException("...");
default:
return new MyDomainHTTPException("...");
}
}
That said, returning an exception in Java doesn't feel right.
It works fine but it doesn't seem absolutely correct because of the "special" status of exceptions. It can be justifiable when there is lazy evaluation, and you will be throwing the exception when you meet a certain condition in the future, or in cases with Optional.orElseThrow
.
It might be confusing to some people who got used to the throw new
template and never thought that returning an exception is a compilable option.
In large frameworks (Spring, PrimeFaces - if my memory serves me correctly), I saw exception factories used to compose exceptions based on the given context and rules. They definitely employ exceptions more extensively than we do. So you may ignore my feeling ;)