Gfrktyl

I have this piece of code placed in ESI class. There is switch statement nested in a try-catch block and it feels simply wrong for me (I have a suspicion that I'm close to exception driven development). I'm looking forward to read your suggestions on how could I potentially improve this code.

@PostConstruct

public void initialize() {

    this.webTarget = this.client.target("http://demo1173642.mockable.io/");

}



public Message getMockMessage() {

    Response response = null;



    try {

        response = this.webTarget.request().get();



        switch (response.getStatusInfo().getFamily()) {

            case SUCCESSFUL:

                return response.readEntity(Message.class);



            case SERVER_ERROR:

                throw new WebApplicationException(ErrorCode.EXTERNAL_SERVICE_BAD_GATEWAY.getMessage(),

                        Response.Status.BAD_GATEWAY);



            default:

                throw new InternalServerErrorException(ErrorCode.EXTERNAL_SERVICE_EXCEPTION.getMessage());

        }



    } catch (Exception ex) {

        logger.log(Level.WARNING, ErrorCode.UNEXPECTED_EXECPTION.getMessage(), ex);

        if (response != null) {

            response.close();

        }

        throw new InternalServerErrorException();

    }

}

In fact, your error handling is quite messed up: in the cases for server error and default you throw an exception, which you immediately replace with a different exception in your own catch block. I don't think that's what you meant to do. Furthermore, you only close() the response object in the exception case, but not in the success branch...

Nevertheless, the throwing of a WebApplicationException leads me to believing that we are in a RESTful application here. Thus, the best way would be to declare an excepton mapper to handle the underlying ProcessingException (which is the only one your catch-all-block should encounter) and do not do the handling internally. For exception mappers, here's a tutorial to start with: https://dennis-xlc.gitbooks.io/restful-java-with-jax-rs-2-0-en/cn/part1/chapter7/exception_handling.html (but you will find a lot more resources using your favourite search engine.)

Having this out of the way, the only thing for your method to do is using a try-with-resources block for the response:

public Message getMockMessage() {

    try(Response response = this.webTarget.request().get()) {

        switch (response.getStatusInfo().getFamily()) {

            case SUCCESSFUL:

                return response.readEntity(Message.class);

            case SERVER_ERROR:

                throw new WebApplicationException(ErrorCode.EXTERNAL_SERVICE_BAD_GATEWAY.getMessage(),

                        Response.Status.BAD_GATEWAY);

            default:

                throw new InternalServerErrorException(ErrorCode.EXTERNAL_SERVICE_EXCEPTION.getMessage());

    }

}