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.

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?
            }
        }
    }
share|improve this question
    
Can you give us us case for this? Why can't you handle every error type in the top level catch? –  Morons Nov 9 '11 at 16:43
1  
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. –  Mister Smith Nov 9 '11 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 '11 at 17:50
1  
Why should it be an anti-pattern? –  user1249 Apr 9 '12 at 21:40
    
+1 for "more try catches?" –  JoelFan Apr 9 '12 at 22:11
add comment

10 Answers

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

Not pretty, but sometimes there are no alternatives.

share|improve this answer
    
Well, you could do the try-catch one after another, without nesting. –  Mister Smith Nov 9 '11 at 16:30
7  
@MisterSmith - Not always. –  Oded Nov 9 '11 at 16:31
2  
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 '11 at 16:40
1  
@MisterSmith: I'd prefer nested try-catches to sequential try-catches that are partially controlled with flag variables (if they were functionally the same). –  FrustratedWithFormsDesigner Nov 9 '11 at 17:11
19  
try{ transaction.commit(); } catch { try{ transaction.rollback(); } catch { seriouslogging() } notsoseriouslogging(); } is an example of a necessary nested try catch –  Thanos Papathanasiou Nov 9 '11 at 17:16
show 7 more comments

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

Most nested try catch's are indeed avoidable and ugly us hell, usually the product of junior developers.

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...

share|improve this answer
add comment

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"
share|improve this answer
add comment

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.

share|improve this answer
add comment

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();
share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
1  
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. –  Hanno Binder Nov 9 '11 at 18:59
add comment

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.

share|improve this answer
add comment

Yes, if you use them like this. :-)

private ParseResult<Statement> parseStatement(int p) {
    try {
        return (ParseResult) parseIfElseStm(p);
    } catch (ParseException e1) {
    try {
        return (ParseResult) parseIfStm(p);
    } catch (ParseException e2) {
    try {
        return (ParseResult) parseWhileStm(p);
    } catch (ParseException e3) {
    try {
        return (ParseResult) parseIterationStm(p);
    } catch (ParseException e4) {
    try {
        return (ParseResult) parseForStm(p);
    } catch (ParseException e5) {
    try {
        return (ParseResult) parseReturnStm(p);
    } catch (ParseException e6) {
    try {
        return (ParseResult) parseLocalDefStm(p);
    } catch (ParseException e7) {
    try {
        return (ParseResult) parseExpStm(p);
    } catch (ParseException e8) {
    try {
        return (ParseResult) parseBlockStm(p);
    } catch (ParseException e9) {
    try {
        // Try the empty statement
        parseChar(p++, ';');
        return new ParseResult<Statement>(EmptyStm.INST, p);
    } catch (ParseException e10) {
        throw new ParseException("expecting statement");
    }}}}}}}}}}
}
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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