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.


Posted

in

by

Tags:

Comments

5 responses to “Exception Handling – The Catch Block should go Last”

  1. Jens Schauder Avatar

    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.

    1. felix Avatar
      felix

      Sure. That’s valid, but in that case you wouldn’t need anything after the catch block anyways.

  2. Nathan Avatar
    Nathan

    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. B. K. Oxley (binkley) Avatar

    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. Martin Anderson Avatar

    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 *

This site uses Akismet to reduce spam. Learn how your comment data is processed.