HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

Exception handling for HTTP request timer

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingexceptionrequesttimerhttpfor

Problem

This method is used to check a URL and return the time it takes to check and the HTTP status code:

public static String urlCheck(String url) {
    try {
        URL urlToCheck = new URL(url);
        HttpURLConnection connection = (HttpURLConnection) urlToCheck.openConnection();
        connection.setRequestMethod("GET");
        Long start = System.currentTimeMillis();
        connection.connect();
        int urlStatus = connection.getResponseCode();
        Long stop = System.currentTimeMillis();
        Long urlCheckTime = stop - start;
        return (connection.getRequestMethod() + " " + urlToCheck + " "
                + urlStatus + " " + urlCheckTime + "ms");
    }
    catch (IOException err){
        logger.log(Level.WARNING,err.getMessage());
        return "Skipping URL";
    }

}


My questions are:

  • Should my try block contain all the lines that are in it now, or should it only contain the line that can throw an exception (I put them all in since variables were not resolving (e.g. urlToCheck on first line was not passed to the second line if the second line was outside the try block))?



  • Is using the blanket exception IOException ok, or would it be better to create a catch block for each exception type that the try block can throw?

Solution

Generally, you should try and minimize the number of statements in a try block to the minimum possible.

However, in your case, since

URL urlToCheck = new URL(url);
HttpURLConnection connection = (HttpURLConnection) urlToCheck.openConnection();
connection.setRequestMethod("GET");


and

connection.connect();
int urlStatus = connection.getResponseCode();


can all throw exceptions, and the rest of the code is relatively minor (5 lines in your version or 4 in mine), I would leave it all as is, in the try block.

I would recommend eliminating stop and writing the timing code like this:

Long start = System.currentTimeMillis();
int urlStatus = connection.getResponseCode();
Long urlCheckTime = System.currentTimeMillis() - start;


This will yield slightly more accurate timing and eliminates a unneeded variable. You can also eliminate urlCheckTime by having the code in the return string but that's a matter of taste.

As for whether to use a blanket IOException or to have individual catch blocks for MalformedUrlException and so on, that is a question that you need to answer. If you want to have more detailed output, you will need the additional catch blocks. If however, you are simply testing to see if the URL is working, then your method is fine.

Code Snippets

URL urlToCheck = new URL(url);
HttpURLConnection connection = (HttpURLConnection) urlToCheck.openConnection();
connection.setRequestMethod("GET");
connection.connect();
int urlStatus = connection.getResponseCode();
Long start = System.currentTimeMillis();
int urlStatus = connection.getResponseCode();
Long urlCheckTime = System.currentTimeMillis() - start;

Context

StackExchange Code Review Q#60598, answer score: 5

Revisions (0)

No revisions yet.