Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

This question already has an answer here:

Note on the question: this is not a duplicate, Efficient try / catch block usage? was asked after this one. The other question is the duplicate.

I was wondering what was the best way to use try/catch. Is it better to limit the content of the try block to the minimum or to put everything in it?

Let me explain myself with an example:

Code 1:

try {
  thisThrowsAnException();
}
catch (Exception e) {
  e.printStackTrace();
}
thisDoesnt();

Code 2:

try {
  thisThrowsAnException();
  thisDoesnt();
}
catch (Exception e) {
  e.printStackTrace();
}

Assuming that thisThrowsAnException()... well... can throw an exception and thisDoesnt()... I'm sure you got it.

I know the difference between the two examples: in case the exception is caught, thisDoesnt() will be called in the first case, not in the second. As it doesn't matter for me, because the throwing of that exception would mean the application should stop, why would one use a version better than the other?

share|improve this question

marked as duplicate by gnat, GlenH7, Rein Henrichs, Jalayn, Robert Harvey Apr 29 '13 at 23:29

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

    
@Michael Thanks for the tag, it's sometimes difficult to identify them. I tried [try-catch] but it didn't exist ;-) –  Oltarus Jun 9 '11 at 8:43
    
@Otarus - no problem :) –  Michael Jun 9 '11 at 9:25
    
I don't like the catch block. Why catch the exception only to print a stack trace, and then continue execution like nothing happened? That just fills your code with catch blocks. Let the exception propagate to the outer loop where something sensible can be done. –  kevin cline Jun 9 '11 at 12:38
    
@kevin Tthis example is not real... I usually put more important stuff in a catch block. –  Oltarus Jun 9 '11 at 12:48
1  
@Oltarus: I thought that might be the case. I try to limit catch blocks to an absolute minimum, only handling the ones I can truly recover from. Otherwise I just let them propagate and catch them all in one place. –  kevin cline Jun 9 '11 at 13:21
show 1 more comment

8 Answers 8

up vote 9 down vote accepted

Like Michael already mentioned, it's not only a question of coding style or best practice, you have to keep the algorithm in mind.

The advantage of code 1 is maintainablity. The try/catch wraps only that one line of code that may throw the exception. So if we see a try/catch block, it's immediatly cleary, which operation can throw it. This is quite valuable if we have a series of operations that throw various IO-Exceptions. And if we remove a statement or refactor some part of the code into a separate method, then we don't lose our exception handlers or leave unnecessary try/catch handlers (of catched runtime exception).

The advantage of code 2 is readabilty. To many try/catch blocks really clutter a method and it becomes extremely difficult to see the real algorithm behind all those try/catch statements and handlers.

In my own code I prefer version 2, the version that has a higher chance to lead to clean code. Even though, I have to pay more attention during refactoring.


Trivia - what you shouldn't put in a catch block is a e.printStackTrace(); statement ;) I'd really recommend to add log4j (or something similiar) and change the IDE's templates to initialize the catch block with log.debug(e)

share|improve this answer
    
Thanks for the trivia! –  Oltarus Jun 9 '11 at 7:19
1  
+1 for suggesting the use of logging framework such as log4j instead of printing the error stacktrace. Though you should use log.error(…) or log.warn(…) instead depending on the severity of the exception. –  Spoike Jun 9 '11 at 8:53
    
@Spoike - thought about the log level and decided for DEBUG as a default for the template. We can/should change the log level and add a custom message if needed after the try/catch block has been autogenerated by the IDE. It was my personal vision, that if we don't implement a catch block actively, the message is more DEBUG then anything else. –  Andreas_D Jun 9 '11 at 9:44
    
@Andreas_D: Thought about it too for a while before you commented and I agree, it'd be a good idea to do log.debug as default for templates. –  Spoike Jun 9 '11 at 10:13
    
I don't like accepting only one single answer when I'm asking a question about opinions, but this one sums up the pro/con and gives a global view of the answer... –  Oltarus Jun 10 '11 at 7:10
show 1 more comment

In my opinion you should put everything in the block that is dependent on the part that throws the exception. So if in your second example:

try {
  thisThrowsAnException();
  thisDoesnt();
}
catch (Exception e) {
  e.printStackTrace();
}

If thisDoesnt(); is dependent of a successful execution of thisThrowsAnException() it should be included.

Does it make sense to run it if thisThrowsAnException() fails?

share|improve this answer
2  
Good point! No, it doesn't make sense. –  Oltarus Jun 9 '11 at 6:40
add comment

It is important to note that by catching an exception you are undertaking the responsibility of recovering from it. The scope of the try block should depend on how that recovery is to be performed. For example, look at these two code snippets:

Snippet 1:

for (Object object : objects) {
    try {
        performTaskOnObject(object);
    } catch (Exception e) {
        log.error("Failed to perform task on object", e);
    }
}

Snippet 2:

try {
    for (Object object : objects) {
        performTaskOnObject(object);
    }
} catch (Exception e) {
    log.error("Failed to perform task on objects", e);
}

By simply changing the scope of the try block, these two snippets present two very different recovery strategies. Snippet 1 allows the task to be attempted for the remaining Objects even after one or more failures. Snippet 2 does not allow the task to be attempted for the remaining Objects after a failure.

Your recovery strategy should be your guide to scoping your try block.

share|improve this answer
    
+1 Actually that's a very good example. –  Songo May 4 '12 at 17:54
add comment

I would prefer the first version, so it becomes clear for the human reader which part of the program throws the exception and how it is handled. In a real world program, thisDoesnt() would be several lines of code, and by pure luck, one of them might throw exceptions too, but version 2 of the code would hide that fact, so the exception handling might do the wrong thing because it's meant to handle a failure of thisThrowsAnException();. And those several lines of thisDoesnt() seperate the exception handler from thisThrowsAnException();, making it harder to see the connection.

Therefore, my version of the code would look like that

boolean success=false;
try {
  thisThrowsAnException();
  success=true;
}
catch (Exception e) {
  e.printStackTrace();
}

if (success) {
  thisDoesnt();
}

or with an early exit in case of an error:

try {
  thisThrowsAnException();
}
catch (Exception e) {
  e.printStackTrace();
  return null;  // false, "", whatever; or rethrow the exception
}

thisDoesnt();
share|improve this answer
add comment

Note there is two ways to implement "the application should stop".

  1. Throwing an exception and let it bubble all the way up to a central error handler that do what you need to have done and then exits the program (or try again or whatever).

  2. Calling "System.exit(1)". This is rarely what you want, except in the simplest programs, and even rarer in e.g. a web application where a simple error could crash the web server. Also, resources are not released properly, etc.

I would strongly recommend learning how to use exceptions to indicate and recover from errors instead of using System.exit(). The finally clause is also very important for cleaning up resources.

In your question, if thisDoesnt() should be executed regardless whether thisThrowsAnException() fails or not, then it is code 1. If it only should be run if not failing, then code 2.

share|improve this answer
add comment

The two codes are doing different things in presence of exception. There is no preference with one or the other, use the one which does what is needed.

share|improve this answer
    
I know, I was asking for personnal and subjective preferences. –  Oltarus Jun 9 '11 at 9:37
    
@Oltarus, your personal preferences may influence the behavior you want, but when that one is fixed, the code structure is determined by the behavior. –  AProgrammer Jun 9 '11 at 9:51
add comment

In my opinion, there can be two conditions,one that we want to execute the program evenafter exception occurs and the next we don't. The later one is simple just log in the errow or give a proper definition that exception that has occured and run away.

In first case:

Lets make it simple to understand

We are writing a program to add list or numbers like (a1/x)+(a1/x)+(a3/x)+...

like this:

try {
  sum=(a1/x)+(a1/x)+(a3/x)+...;
}
catch (Exception e) {
  e.printStackTrace();
}
syso.(sum);

Now if somebody inserted x=0 then an exception occurs. Now do you want the program to end like this?

we would than want to write like this

try {
  sum=(a1/x)+(a1/x)+(a3/x)+...;
  syso.(sum);
}
catch (Exception e) {
  e.printStackTrace();//optional
  syso("Hey man! You cannot divideanything by zero!!! Its infinity");//something like this
}

or to be more clear:

try {
  sum=(a1/x)+(a1/x)+(a3/x)+...;
}
catch (Exception e) {
    syso("Hey man! You cannot divideanything by zero!!! Its infinity");//something like this
    sum=(a1/y)+(a1/y)+(a3/y)+...;
}
syso(sum);

Note the where syso(sum) is placed

Hope it gives you a bit clear idea :)

share|improve this answer
add comment

Its going to be a game-time decision for each one. If you have code that can throw exceptions that is not absolutely necessary for the program, then put it alone in the try block. That way in the catch block you can deal with where to go next.

A good design pattern for this is to have a helper method which can determine if an error is fatal (end the session, close the program, whatever) or not, and then rethrow or not based on the result of that check. Something like:

try{
    foo();
}
catch(Exception ex)
{
    bool rethrow = ExceptionHelper.Handle(ex);  //should return true if error is fatal, false if not
    if(rethrow) throw;
}
share|improve this answer
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.