19

Is there a "best practice" for how much code to put inside a try/catch block?

I have posted 3 different scenarios below.

I did not include behavior in each catch block and i did not include the finally block. This was to improve readability for viewers. Assume each catch does something differently. And assume the finally will be closing the stream. Just trying to create an easy to read example for future readers.

  1. Control, no try/catch.
  2. Code with 1 try/catch for each place needed.
  3. Code with only 1 try/catch surrounding whole code block.

What is generally accepted as the best practice and why?


Scenario 1

Code without try/catch, just for control.

 BufferedReader bufferedReader = new BufferedReader(new FileReader("somepath")); String line; while ((line = bufferedReader.readLine()) != null) { Object object = new Object(); this.doSomething(object); } bufferedReader.close(); 

Scenario 2

Code with a try/catch block for each individual place needed.

 BufferedReader bufferedReader = null; try { bufferedReader = new BufferedReader(new FileReader("somepath")); } catch (FileNotFoundException e) { e.printStackTrace(); } String line; try { while ((line = bufferedReader.readLine()) != null) { Object object = new Object(); this.doSomething(object); } } catch (IOException e) { e.printStackTrace(); } try { bufferedReader.close(); } catch (IOException e) { e.printStackTrace(); } 

Scenario 3

Code with 1 try/catch surrounding the whole block of code.

 try { BufferedReader bufferedReader = new BufferedReader(new FileReader("somepath")); String line; while ((line = bufferedReader.readLine()) != null) { Object object = new Object(); this.doSomething(object); } bufferedReader.close(); } catch (FileNotFoundException e) { e.printStackTrace(); } catch (IOException e) { e.printStackTrace(); } 
9
  • 2
    I think it's better to put one try statement and multiple catches for the exceptions you want to catch. Better for readability. Commented Oct 24, 2013 at 15:13
  • Within scenario 1 do you mean where the method itself throws IOException Commented Oct 24, 2013 at 15:13
  • @Vallentin Scenario 1 is just for control, basically ignore it. I just wanted to show the code with no try/catch blocks. Commented Oct 24, 2013 at 15:14
  • @AliAlamiri I agree with Scenario 3 being much easier to read, but is that the only reason why? Commented Oct 24, 2013 at 15:15
  • 2
    @prolink007 Part of best practice is related to readability. There will be other factors, which depend on the code itself and what is being used (my opinion). Scenario 3 may not be good in other scenarios for example. Commented Oct 24, 2013 at 15:17

10 Answers 10

16

You should scope your try/catches based on the following criteria:

  • do you need to do different things based on where the exception came from?
  • do you need to do different things based on which exception was thrown?
  • what code needs to be skipped (aka is "invalid") when a given exception is thrown?

answering these questions will enable you to determine the appropriate scope for any try/catch block.

Sign up to request clarification or add additional context in comments.

3 Comments

@Basilevs - mention what specifically about nesting?
There are scenarios which require nested try catch blocks, I've thought that minor point might make this answer better. I was probably wrong - the structure would be corrupted.
@Basilevs - nested try/catch blocks are an implementation choice which relate back to the points i mentioned above. answering those questions should tell you how you need to specify your try/catch blocks (single/multiple/nested/etc).
8

I'd put as little in the try-catch as possible.

This allows you to very easily move pieces of code into separate methods which is a good coding practice (obeying single responsibility principle; see the book "Clean Code: A Handbook of Agile Software Craftsmanship" by Robert C. Martin).

Another advantage is that you can quickly identify which code actually can throw an exception.

Scenario 2 seems a bit extreme though, and since the method is pretty small, Scenario 3 seems like the best choice.

However, you need to have your "close" statement in a finally block.

1 Comment

I was just trying to create an example. I had not included behavior for each catch and did not include the finally. Just trying to keep the examples very easy for users to just skim through. Did not want to give information overload.
2

This is a matter of opinion. I've seen each of these patterns a lot.

Pattern 1 is only good when your method can throw the excpetions and have something up the caller chain handle that. That is often desireable. However, since the close call is not in a finally block, it might not be called. At least, use a try-finally block.

Pattern 2 isn't good, since if the first try-catch block handles an exception, the rest of the method is useless.

Pattern 3 is okay, but not great, as printing stack traces hides the fact that the operation failed. What will the caller do if it thinks the operation occurred when it didn't. Also, the closes might not have happened, which can lead to program failure.

In pseudo code, this variant of pattern 3 is better:

Declare Streams, connections, etc. try Initialize streams, connections, etc, Do work. catch (optional) Catch and handle exceptions. Do not simply log and ignore. finally Close connections and streams in reverse order. Remember, closing these objects can throw, so catch exceptions the close operation throws. End. 

If you are using Java 7, use try-with-resources:

try (BufferedReader bufferedReader = new BufferedReader(new FileReader("somepath"))) { String line; while ((line = bufferedReader.readLine()) != null) { Object object = new Object(); this.doSomething(object); } } 

Have IOExceptions bubble up to the caller.

1 Comment

I was just trying to create an example. I had not included behavior for each catch and did not include the finally. Just trying to keep the examples very easy for users to just skim through. Did not want to give information overload.
1

You should go with the third scenario.

If the bufferedReader hits an exception then on creation in the second scenario then it you try to readLine() on it, it will hit another exception. No point in raising multiple exceptions for the same problem.

You should also close your bufferedReader in the finally block.

BufferedReader bufferedReader; try { bufferedReader = new BufferedReader(new FileReader("somepath")); String line; while ((line = bufferedReader.readLine()) != null) { Object object = new Object(); this.doSomething(object); } } catch (FileNotFoundException e) { e.printStackTrace(); } catch (IOException e) { e.printStackTrace(); } finally { if (bufferedReader != null) bufferedReader.close(); } 

2 Comments

I was just trying to create an example. I had not included behavior for each catch and did not include the finally. Just trying to keep the examples very easy for users to just skim through. Did not want to give information overload.
Also, close() can throw.
1

I consider an Exception being an uncoditionally returned result type. So when I work with try-catch sections, I'm trying to answer questions

  1. Should I handle an unexpected result here or should I propagate it on a higher level?
  2. Which unexpected results should I handle?
  3. How to handle them?

In 95% of the cases I don't go further than the first point, so I just propagate the errors.

For file processing I use try-with-resources an re-throw the IOException with throw new RuntimeException(e).

Comments

0

I think it's similar with how much code to put in method. Try to write try/catch/finally which takes no more than one screen. I want to say it's not a problem to wrap entire method body into try{} block, but you should separate this code into several methods if it became too long.

Comments

0

You example with one try/catch each doesn't make sense because you just print a stack trace and continue - knowing already that it will be a failure. You might as well try/catch the whole lot, or add throws SomeException to the method signature and let the calling method decide what went wrong.

Also, don't worry about squeezing too much inside a try/catch. You can always extract that code into another method. Readability is one of the single most important aspects of programming.

Comments

0

The third option is certainly best. You don't want your try/catch block to become unwieldy, but in this example, it is short enough that you do not need to divide it as you have done in your second option.

Comments

0

Not scenario 2. If the FileReader or BufferedReader constructor throws, then bufferedReader will be null but the next try/catch will still execute. Therefore you'll get an (uncaught) exception at bufferedReader.readLine -- a NullPointerException. Between scenarios 1 and 3, I generally like 3 more because it doesn't require the caller to catch. BTW, you don't need to catch FileNotFoundException explicitly, because it inherits from IOException and therefore that catch block will catch both.

Comments

0

jtahlborn already has the correct answer.

Unfortunately sometimes, proper exception handling can be pretty bloated. One should be ready to deal with it, if required.

Another possible scenario are nested try-catch blocks.

Consider:

BufferedReader bufferedReader = new BufferedReader(new FileReader("somepath")); try { String line; while ((line = bufferedReader.readLine()) != null) { Object object = new Object(); try { this.doSomething(object); } catch (InvalidArgumentException iae) { throw new RuntimeErrorException("Failed to process line " + line + ", iae); } catch (ParserWarning e) { e.printStackTrace(); } } } catch (IOException e) { e.printStackTrace(); } finally { bufferedReader.close(); } 

1 Comment

Yeah, it can easily become much more complicated. I was just trying to give a simple example that easy to read for other viewers.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.