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

23. February 2011

Software Development

Felix Leipold

Tags:

Comments

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

Jens Schauder

23. February 2011

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.

Reply
1. felix
  
  24. February 2011
  
  Sure. That’s valid, but in that case you wouldn’t need anything after the catch block anyways.
  
  Reply
Nathan

24. February 2011
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;
    }
```
Reply

B. K. Oxley (binkley)

24. February 2011

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.

Martin Anderson

8. April 2011

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]

Reply

Exception Handling – The Catch Block should go Last

Comments

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

Leave a Reply Cancel reply