161

Is this an antipattern? It is an acceptable practice?

    try {
        //do something
    } catch (Exception e) { 
        try {
            //do something in the same line, but being less ambitious
        } catch (Exception ex) {
            try {
                //Do the minimum acceptable
            } catch (Exception e1) {
                //More try catches?
            }
        }
    }
7
  • Can you give us us case for this? Why can't you handle every error type in the top level catch?
    – Morons
    Nov 9, 2011 at 16:43
  • 3
    I've seen this kind of code recently, performed by unexperienced programmers who don't really know what they are calling inside the try blocks, and they don't want to bother testing the code. In the code sample I've seen, It was the same operation but performed each time with fallback parameters. Nov 9, 2011 at 16:50
  • @LokiAstari -Your example is a try in the Finally Section.. Where there is no Catch. This is a nested in the Try section.. It's different.
    – Morons
    Nov 9, 2011 at 17:50
  • 6
    Why should it be an anti-pattern?
    – user1249
    Apr 9, 2012 at 21:40
  • 3
    +1 for "more try catches?"
    – JoelFan
    Apr 9, 2012 at 22:11

13 Answers 13

132

This is sometimes unavoidable, especially if your recovery code might throw an exception.

Not pretty, but sometimes there are no alternatives.

9
  • if you do the try catch one after another then surely the code inside the second try will be executed even if the first catch clause is not invoked?
    – AndrewC
    Nov 9, 2011 at 16:35
  • 4
    Yes this is sort of what I was trying to get at. Of course there comes a point in your nested try/catch statements where you just have to say enough is enough. I was making a case for nesting as opposed to sequential try/catch's, saying that there are situations where you only want the code inside the second try to execute if the first try falls over.
    – AndrewC
    Nov 9, 2011 at 16:40
  • 8
    @MisterSmith: I'd prefer nested try-catches to sequential try-catches that are partially controlled with flag variables (if they were functionally the same). Nov 9, 2011 at 17:11
  • 46
    try{ transaction.commit(); } catch { try{ transaction.rollback(); } catch { seriouslogging() } notsoseriouslogging(); } is an example of a necessary nested try catch Nov 9, 2011 at 17:16
  • 5
    At least extract the catch block to a method, guys! Let's at least make this readable.
    – Mr Cochese
    Jun 25, 2014 at 18:01
67

I don't think its an antipattern, just widely misused.

Most nested try catch's are indeed avoidable and ugly as hell, but there are times you can't help it.

try {
    transaction.commit();
} catch {
    logerror();
    try {
        transaction.rollback();
    } catch {
        seriousLogging();
    }
}

Also, you'll need an extra bool somewhere to signify the failed rollback...

27

It's okay. A refactoring to consider is pushing the code into its own method, and using early exits for success, letting you write the different attempts to do something at the same level:

try {
    // do something
    return;
} catch (Exception e) {
    // fall through; you probably want to log this
}
try {
    // do something in the same line, but being less ambitious
    return;
} catch (Exception e) {
    // fall through again; you probably want to log this too
}
try {
    // Do the minimum acceptable
    return;
} catch (Exception e) {
    // if you don't have any more fallbacks, then throw an exception here
}
//More try catches?

Once you have it broken out like that, you could think about wrapping it up in a Strategy pattern.

interface DoSomethingStrategy {
    public void doSomething() throws Exception;
}

class NormalStrategy implements DoSomethingStrategy {
    public void doSomething() throws Exception {
        // do something
    }
}

class FirstFallbackStrategy implements DoSomethingStrategy {
    public void doSomething() throws Exception {
        // do something in the same line, but being less ambitious
    }
}

class TrySeveralThingsStrategy implements DoSomethingStrategy {
    private DoSomethingStrategy[] strategies = {new NormalStrategy(), new FirstFallbackStrategy()};
    public void doSomething() throws Exception {
        for (DoSomethingStrategy strategy: strategies) {
            try {
                strategy.doSomething();
                return;
            }
            catch (Exception e) {
                // log and continue
            }
        }
        throw new Exception("all strategies failed");
    }
}

Then just use the TrySeveralThingsStrategy, which is a kind of composite strategy (two patterns for the price of one!).

One huge caveat: don't do this unless your strategies are themselves sufficiently complex, or you want to be able to use them in flexible ways. Otherwise, you're larding a few line of simple code with a huge pile of unnecessary object-orientation.

21

The logic is fine - it can make perfect sense in some situations to try a fallback approach, which could itself experience exceptional events.... hence this pattern is pretty much unavoidable.

However I would suggest the following to make the code better:

  • Refactor the inner try...catch blocks out into separate functions, e.g. attemptFallbackMethod and attemptMinimalRecovery.
  • Be more specific about the particular exception types that are being caught. Do you really expect any Exception subclass and if so do you really want to handle them all the same way?
  • Consider whether a finally block might make more sense - this is usually the case for anything that feels like "resource cleanup code"
1
  • Refactoring into a separate function doesn’t change the fact there is a nested try-catch - it just hides it, brushes it under the carpet.
    – gnasher729
    Nov 17, 2023 at 8:29
9

I don't think it's automatically an anti-pattern, but I'd avoid it if I can find an easier and cleaner way to do the same thing. If the programming language you're working in has a finally construct, that might help clean this up, in some cases.

9

Not an anti-pattern per se, but a code pattern that tells you need to refactor.

And it's pretty easy, you just have to know a rule of thumb which is writing no more than a try block in the same method. If you know well to write related code together, usually is just copying and pasting each try block with its catch blocks and pasting it inside a new method, and then replace the original block with a call to this method.

This rule of thumb is based on Robert C. Martin's suggestion from his book 'Clean Code':

if the keyword 'try' exists in a function, it should be the very first word in the function and that there should be nothing after the catch/finally blocks.

A quick example on "pseudo-java". Suppose we have something like this:

try {
    FileInputStream is = new FileInputStream(PATH_ONE);
    String configData = InputStreamUtils.readString(is);
    return configData;
} catch (FileNotFoundException e) {
    try {
        FileInputStream is = new FileInputStream(PATH_TWO);
        String configData = InputStreamUtils.readString(is);
        return configData;
    } catch (FileNotFoundException e) {
        try {
            FileInputStream is = new FileInputStream(PATH_THREE);
            String configData = InputStreamUtils.readString(is);
            return configData;
        } catch (FileNotFoundException e) {
            return null;
        }
    }
}

Then we could refactor each try catch and in this case each try-catch block tries the same thing but in different locations (how convenient :D), we have only to copy paste one of the try-catch blocks and make a method of it.

public String loadConfigFile(String path) {
    try {
        FileInputStream is = new FileInputStream(path);
        String configData = InputStreamUtils.readString(is);
        return configData;
    } catch (FileNotFoundException e) {
        return null;
    }
}

Now we use this with the same purpose as before:

String[] paths = new String[] {PATH_ONE, PATH_TWO, PATH_THREE};

String configData;
for(String path : paths) {
    configData = loadConfigFile(path);
    if (configData != null) {
        break;
    }
}
1
  • good example. this example is truly the kind of code we have to refactor. however some other times, nested try-catch is necessary.
    – linehrr
    Jun 3, 2018 at 22:49
6

It is surely decreasing the code readability. I would say, if you have chance, then avoid nesting try-catches.

If you have to nest try-catches, always stop for a minute and think:

  • do I have the chance to combine them?

    try {  
      ... code  
    } catch (FirstKindOfException e) {  
      ... do something  
    } catch (SecondKindOfException e) {  
      ... do something else    
    }
    
  • should I just simply extract the nested part into a new method? The code will be much more clean.

    ...  
    try {  
      ... code  
    } catch (FirstKindOfException e) {  
       panicMethod();  
    }   
    ...
    
    private void panicMethod(){   
    try{  
    ... do the nested things  
    catch (SecondKindOfException e) {  
      ... do something else    
      }  
    }
    

It is obvious if you have to nest three or more levels of try-catches, in a single method, that is a sure sign of time for refactor.

3

I've seen this pattern in network code, and it actually makes sense. Here's the basic idea, in pseudocode:

try
   connect;
catch (ConnectionFailure)
   try
      sleep(500);
      connect;
   catch(ConnectionFailure)
      return CANT_CONNECT;
   end try;
end try;

Basically it's a heuristic. One failed attempt to connect could just be a network glitch, but if it happens twice, that probably means the machine you're trying to connect to really is unreachable. There are probably other ways to implement this concept, but they'd most likely be even uglier than the nested tries.

2

I solved this situation like this (try-catch with fallback):

$variableForWhichINeedFallback = null;
$fallbackOptions = array('Option1', 'Option2', 'Option3');
while (!$variableForWhichINeedFallback && $fallbackOptions){
    $fallbackOption = array_pop($fallbackOptions);
    try{
        $variableForWhichINeedFallback = doSomethingExceptionalWith($fallbackOption);
    }
    catch{
        continue;
    }
}
if (!$variableForWhichINeedFallback)
    raise new ExceptionalException();
2

I've "had" to do this in a test class coincidentially (JUnit), where the setUp() method had to create objects with invalid constructor parameters in a constructor that threw an exception.

If I had to make the construction of 3 invalid object fail, for example, I would need 3 try-catch blocks, nested. I created a new method instead, where the exceptions where caught, and the return value was a new instance of the class I was testing when it succeeded.

Of course, I only needed 1 method because I did the same 3 times. It might be not such a good solution for nested blocks that do totally different things, but at least your code would become more readable in most cases.

0

I actually think it's an antipattern.

In some cases you might want multiple try-catches, but only if you DONT KNOW what kind of error you are looking, eg:

public class Test
{
    public static void Test()
    {            
        try
        {
           DoOp1();
        }
        catch(Exception ex)
        {
            // treat
        }

        try
        {
           DoOp2();
        }
        catch(Exception ex)
        {
            // treat
        }

        try
        {
           DoOp3();
        }
        catch(Exception ex)
        {
            // treat
        }
    }

    public static void Test()
    {
        try
        {
            DoOp1();
            DoOp2();
            DoOp3();
        }
        catch (DoOp1Exception ex1)
        {
        }
        catch (DoOp2Exception ex2)
        {
        }
        catch (DoOp3Exception ex3)
        {
        }
    }
}

If you don't know what you're looking for you HAVE to use the first manner, which is IMHO, ugly and not functional. I guess the latter is much better.

So, if you know what KIND of error you are looking for, be specific. No need for nested or multiple try-catches inside the same method.

1
  • 2
    Code like the one you showed does indeed not make any sense in most if not all cases. However, the OP was referring to nested try-catches, which is a quite different question than that for multiple consecutive statements.
    – JimmyB
    Nov 9, 2011 at 18:59
0

In some cases a nested Try-Catch is unavoidable. For instance when the error recovery code itself can throw and exception. But in order to improve the readability of the code you can always extract the nested block into a method of its own. Check out this blog post for more examples on nested Try-Catch-Finally blocks.

0

There nothing mentioned as Anti Pattern in java anywhere. Yes we call few things good practice and bad practice.

If a try/catch block is required inside a catch block its required you cant help it. And there is no alternative. As a catch block can't work as try part if exception is thrown.

For Example :

String str=null;
try{
   str = method(a);
}
catch(Exception)
{
try{
   str = doMethod(a);
}
catch(Exception ex)
{
  throw ex;
}

Here in the above example method throws exception but the doMethod (used for handling method exception) even throw exception. In this case we have to use the try catch within try catch.

some thing which is suggested not to do is ..

try 
{
  .....1
}
catch(Exception ex)
{
}
try 
{
  .....2
}
catch(Exception ex)
{
}
try 
{
  .....3
}
catch(Exception ex)
{
}
try 
{
  .....3
}
catch(Exception ex)
{
}
try 
{
  .....4
}
catch(Exception ex)
{
}
3
  • this doesn't seem to offer anything substantial over prior 12 answers
    – gnat
    Sep 29, 2015 at 19:53
  • In fact, the code which you said "not to do" is sometimes the correct/better choice. It just needs to be structured properly, e.g. with each try-catch section moved to its own method. Whether it is good or bad depends on the level of essential complexity (unavoidable code complexity) resulting from complexity in requirements. In non-transactionally-oriented (e.g. non-CRUD) software systems, exception handling can evolve into something like a state-machine; it is apparent that each action within each state has its try-catch block so that the state machine itself is protected.
    – rwong
    Oct 26, 2020 at 22:25
  • Of course, any advice applied to the extreme can become bad. If a programmer converts each line of code into a try-catch block, it will look excessive to the extreme to the code reviewer. Though, sometimes this is exactly what framework does: each single callback (from framework code to user-defined code) is wrapped in a try-catch. It is just hidden from view, because each try-catch block sits in its own method.
    – rwong
    Oct 26, 2020 at 22:27

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