Question

Currently there is a method that loads an image from an URL and then processes it like follows:

BufferedImage tempImage;
try {
    tempImage = ImageIO.read(url);
}
catch (IOException e) {
    return;
}
final BufferedImage image = tempImage;

To clean the code up a bit, I would like to extract the try/catch block while preserving the functionality of exiting the method if the loading of the image fails. I have tried extracting the block as follows:

final BufferedImage image = loadImageFromURL();
if (image == null)
    return;

//--------------------------------------

private BufferedImage loadImageFromURL() {
    BufferedImage tempImage = null;
    try {
        tempImage = ImageIO.read(url);
    }
    catch (IOException e) {
        LogUtils.severe(e);
    }
    return tempImage;
}

The problem is that if the loading of the image fails, the method returns null, which does not make the code any cleaner, as I have to perform an additional null check. I have read through Clean Code: A Handbook of Agile Software Craftsmanship but all the examples seem to rethrow the exception in some form: is that the only/correct way to go?

Was it helpful?

Solution

We don't know the complete situation you are in, but I would just add a throw clause to the method signature and do not use any try catch.

private BufferedImage loadImageFromURL() throws IOException
{
    return ImageIO.read(url);
}

Which basically resolves back to the original method, without extracting the try-catch. If the loading fails, the method will stop, but not really return. The exception will go all the way back to some piece of code that handles it properly.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top