Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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.

share|improve this question

1 Answer 1

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);
}
share|improve this answer
    
+1 for this common-sense answer. –  kiwiron 22 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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