CODEWITHSUNDEEP
javaexceptionswitch-statementthrow
MarkyMark
MarkyMark

Reputation: 219

Return exception in switch statement and then throw the called method

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..

Upvotes: 3

Views: 1921

Answers (3)

Andrew
Andrew

Reputation: 49606

  1. Get rid of response.getStatusCode() / 100 == 2. Write response.getStatusCode() == 200 or response.getStatusCode() == HttpStatus.SC_OK instead.

  2. Remove the else branch and throw after the if statement.

  3. Rename the method to getExceptionByStatusCode or generateExceptionForStatusCode. You don't handleException, you decide which one to throw.

  4. Choose the right return type. Don't use RuntimeException. It can be ResponseStatusException or any other type appropriate to the HTTP/your domain abstraction.

  5. 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 ;)

Upvotes: 1

John Kugelman
John Kugelman

Reputation: 361615

There's nothing wrong with what you have, but you may find it more natural to immediately throw the exception when it's created:

private String handleResponse(HttpResponse httpResponse){
    if (response.getStatusCode() / 100 == 2) {
        return response.getEntity().toString()
    } else {
        throwException(response.getStatusCode());
    }
}

private void throwException(int errorStatusCode){
    switch(errorStatusCode) {
        case 400:
            throw new RuntimeException("Invalid request");
        case 401:
            throw new RuntimeException("User not authorized");
        default:
            throw new RuntimeException("Unkown exception");
    }
}

The API I'm calling can return as a successful response 200, 201, 204. But I'm open to any suggestions.

If you'll accept any 2xx status code, how about:

if (response.getStatusCode() >= 200 && response.getStatusCode() <= 299) {

Upvotes: 0

Arvind Kumar Avinash
Arvind Kumar Avinash

Reputation: 79085

You can make it a bit shorter and more intuitive in the following way:

private String handleResponse(HttpResponse httpResponse){
    if (response.getStatusCode() == 200) {
        return response.getEntity().toString()
    } else {
        throw new RuntimeException(getErrorMessage(response.getStatusCode());
    }
}

private String getErrorMessage(int errorStatucCode){
    switch(errorStatucCode) {
        case 400:
            return "Invalid request";
        case 401:
            return "User not authorized";
        default:
            return "Unkown exception";
}

Upvotes: 1

Related Questions