Exception Handling – The Catch Block should go Last

I have just stumbled across a piece of code like this:

    Object getSomeValue() {
        Object value = null;
        try {
           value = errorProneOperation();
        } catch (IOException e) {
            throw new IllegalStateException(e);
        }
        return value;
    }

Now I find this really awkward. Why initialise something to null to keep the compiler happy?
Surely you want to do this:

    Object getSomeValue() {
        try {
           Object value = errorProneOperation();
           return value;
        } catch (IOException e) {
            throw new IllegalStateException(e);
        }
    }

No useless initialisation here. Also the scope of the local variable is smaller, which is good. I think there is a general rule here, which is: “There should be nothing after your catch-handler apart from a potential finally”. As with all rules it might be broken, but only with good reason. I don’t want to see that crappy initialisation to null again.

This entry was posted in Software Development. Bookmark the permalink.

5 Responses to Exception Handling – The Catch Block should go Last

  1. In principle I agree, but if you do two things in the try block and want to use ‘value’ in the exception handler if it is set to something you have to use the null initialization.

  2. Nathan says:

    You don’t need the null initialisation, the compiler can figure out you either assign or exit the method via exception. If you are avoiding multiple return statements in method, consistency means you know you can jump to last line of method to see the return. No having to go searching in nested try blocks.

        Object getSomeValue() {
            final Object value;
            try {
               value = errorProneOperation();
            } catch (IOException e) {
                throw new IllegalStateException(e);
            }
            return value;
        }
    
  3. I prefer:

    Object getSomeValue() {
        try {
            return errorProneOperation();
        } catch (IOException e) {
            throw new IllegalStateException(e);
        }
    }

    Or best of all:

    Object getSomeValue() throws IOException {
        return errorProneOperation();
    }

    If the operation is error-prone, the caller should be prepared to handle the situation.

  4. If you have to return something for good practise thought, I can see the need for a declaration (even if the null is not required) rather than multiple returns even though that is shorter, e.g.

    [code]
    List getSomeValue() {
    List value;// as Nathan pointed out, no assignment is necessary
    try {
    value = errorProneOperation();
    } catch (IOException e) {
    value = Collections.emptyList();
    }
    return value;
    }
    [code]

    being preferable to…

    [code]
    List getSomeValue() {
    try {
    return errorProneOperation();
    } catch (IOException e) {
    return Collections.emptyList();
    }
    }
    [code]

Leave a Reply

Your email address will not be published. Required fields are marked *