1
\$\begingroup\$

I wrote some code which simply retrieves the HTTP status code of the passed in URL. There are multiple potential causes of the same error. I am hoping to get some all around feedback on this code, but my primary concern is that I have low readability in an attempt to provide good feedback in the event of an error.

private static final String HOST_NAME = "proxy.mycompany.com";
private static final int PORT = 5555;
private static final Logger LOGGER = LoggerFactory
        .getLogger("HttpStatusCode");

public static int getHttpStatusCode(URL url) {
    Proxy proxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(
                  HOST_NAME, PORT));
    HttpURLConnection connection = null;
    try {
           connection = (HttpURLConnection) url.openConnection(proxy);
    } catch (IOException e) {
           LOGGER.error("URL.openConnection(Proxy) failed in HttpStatusCode.getHttpStatusCode(URL)!");
           LOGGER.error("URL attempted: " + url);
    }
    int responseCode = -1;
    try {
           responseCode = connection.getResponseCode();
    } catch (IOException e) {
           LOGGER.error("HttpURLConnection.getResponseCode() failed in HttpStatusCode.getHttpStatusCode(URL)!");
           LOGGER.error("URL attempted: " + url);
    }
    connection.disconnect();
    return responseCode;
}

My original attempt was the following, but my co-worker advised that I change it to the former for better error handling:

private static final String HOST_NAME = "proxy.mycompany.com";
private static final int PORT = 5555;
private static final Logger LOGGER = LoggerFactory
        .getLogger("HttpStatusCode");

public static int getHttpStatusCode(URL url) {
    Proxy proxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(
            HOST_NAME, PORT));
    try {
        HttpURLConnection connection = (HttpURLConnection) url
                .openConnection(proxy);
        int responseCode = connection.getResponseCode();
        connection.disconnect();
        return responseCode;
    }
    catch (IOException e) {
        LOGGER.error("IOException in HttpStatusCode.getHttpStatusCode(URL)!");
        LOGGER.error("URL attempted: " + url);
        return -1;
    }
}

Just to be clear, this code compiles and runs error free based on my testing. I am looking for a review on the way I am handling potential errors.

\$\endgroup\$
2
\$\begingroup\$

Your log output shouldn't also be detailing the code that failed, because the JVM is going to generate a more detailed stacktrace, and you should print that instead.

LOGGER.error("..." + url, e);

Instead, it will be more beneficial to give a more descriptive message. Borrowing your original code...

try {
       connection = (HttpURLConnection) url.openConnection(proxy);
} catch (IOException e) {
       LOGGER.error("Unable to open a connection to " + proxy, e);
}
int responseCode = -1;
try {
       responseCode = connection.getResponseCode();
} catch (IOException e) {
       LOGGER.error("Could not get response code from " + proxy, e);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.