I have two methods
private String handleResponse(HttpResponse httpResponse){
if (response.getStatusCode() / 100 == 2) {
return response.getEntity().toString()
} else {
throw handleException(response.getStatusCode());
}
}
private RuntimeException handleException(int errorStatusCode){
switch(errorStatusCode) {
case 400:
return new RuntimeException("Invalid request");
case 401:
return new RuntimeException("User not authorized");
default:
return new RuntimeException("Unkown exception");
}
}
Everything works as expected, but is this approach correct? I mean to return new RuntimeException from a switch in a method and then throw the whole method ? Something tells me that it is not and I would like to know why and how can i improve it..
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 ;)