Pattern to avoid nested try catch blocks?

Question

Consider a situation where I have three (or more) ways of performing a calculation, each of which can fail with an exception. In order to attempt each calculation until we find one that succeeds, I have been doing the following:

double val;

try { val = calc1(); }
catch (Calc1Exception e1)
{ 
    try { val = calc2(); }
    catch (Calc2Exception e2)
    {
        try { val = calc3(); }
        catch (Calc3Exception e3)
        {
            throw new NoCalcsWorkedException();
        }
    }
}

Is there any accepted pattern which achieves this in a nicer way? Of course I could wrap each calculation in a helper method which returns null on failure, and then just use the ?? operator, but is there a way of doing this more generally (i.e. without having to write a helper method for each method I want to use)? I've thought about writing a static method using generics which wraps any given method in a try/catch and returns null on failure, but I'm not sure how I would go about this. Any ideas?

Solution 1

As far as possible, don't use exceptions for control flow or unexceptional circumstances.

But to answer your question directly (assuming all the exception-types are the same):

Func<double>[] calcs = { calc1, calc2, calc3 };

foreach(var calc in calcs)
{
   try { return calc(); }
   catch (CalcException){  }
} 

throw new NoCalcsWorkedException();

Solution 2

You could flatten out the nesting by putting it into a method like this:

private double calcStuff()
{
  try { return calc1(); }
  catch (Calc1Exception e1)
  {
    // Continue on to the code below
  }

  try { return calc2(); }
  catch (Calc2Exception e1)
  {
    // Continue on to the code below
  }

  try { return calc3(); }
  catch (Calc3Exception e1)
  {
    // Continue on to the code below
  }

  throw new NoCalcsWorkedException();
}

But I suspect the real design problem is the existence of three different methods that do essentially the same thing (from the caller's perspective) but throw different, unrelated exceptions.

This is assuming the three exceptions are unrelated. If they all have a common base class, it'd be better to use a loop with a single catch block, as Ani suggested.

Solution 3

Just to offer an "outside the box" alternative, how about a recursive function...

//Calling Code
double result = DoCalc();

double DoCalc(int c = 1)
{
   try{
      switch(c){
         case 1: return Calc1();
         case 2: return Calc2();
         case 3: return Calc3();
         default: return CalcDefault();  //default should not be one of the Calcs - infinite loop
      }
   }
   catch{
      return DoCalc(++c);
   }
}

NOTE: I am by no means saying that this is the best way to get the job done, just a different way

Solution 4

Try not to control logic based on exceptions; note also that exceptions should be thrown only in exceptional cases. Calculations in most cases should not throw exceptions unless they access external resources or parse strings or something. Anyway in the worst case follow the TryMethod style (like TryParse()) to encapsulate exception logic and make your control flow maintainable and clean:

bool TryCalculate(out double paramOut)
{
  try
  {
    // do some calculations
    return true;
  }
  catch(Exception e)
  { 
     // do some handling
    return false;
  }

}

double calcOutput;
if(!TryCalc1(inputParam, out calcOutput))
  TryCalc2(inputParam, out calcOutput);

Another variation utilizing the Try pattern and combining list of methods instead of nested if:

internal delegate bool TryCalculation(out double output);

TryCalculation[] tryCalcs = { calc1, calc2, calc3 };

double calcOutput;
foreach (var tryCalc in tryCalcs.Where(tryCalc => tryCalc(out calcOutput)))
  break;

and if the foreach is a little complicated you can make it plain:

        foreach (var tryCalc in tryCalcs)
        {
            if (tryCalc(out calcOutput)) break;
        }

Solution 5

Create a list of delegates to your calculation functions and then have a while loop to cycle through them:

List<Func<double>> calcMethods = new List<Func<double>>();

// Note: I haven't done this in a while, so I'm not sure if
// this is the correct syntax for Func delegates, but it should
// give you an idea of how to do this.
calcMethods.Add(new Func<double>(calc1));
calcMethods.Add(new Func<double>(calc2));
calcMethods.Add(new Func<double>(calc3));

double val;
for(CalcMethod calc in calcMethods)
{
    try
    {
        val = calc();
        // If you didn't catch an exception, then break out of the loop
        break;
    }
    catch(GenericCalcException e)
    {
        // Not sure what your exception would be, but catch it and continue
    }

}

return val; // are you returning the value?

That should give you a general idea of how to do it (i.e. it's not an exact solution).

View more solutions

Comments

[James Johnson]

Can you include some details about the calculation?

[jjoelson]

They're basically just different methods of solving/approximating a PDE. They're from a 3rd party library, so I can't alter them to return error codes or null. The best I could do is wrap each one individually in a method.

[Chris]

Are the calc methods part of your project (instead of a third party library)? If so, you could pull out the logic that throws exceptions and use that to decide which calc method needs to be called.

[Miserable Variable]

There is another use-case for this that I have come across in Java -- I need to parse a String to a Date using SimpleDateFormat.parse and I need to try several different formats in order, moving on to next when one throws exception.

[DeCaf]

How does this help? if calc1() fails cals2 will never be executed!

[DusanV]

this doesn't solve the problem. Only execute calc1 if calc2 fails, only execute calc3 if calc1 && calc2 fail.

[Wyzard]

This assumes that Calc1Exception, Calc2Exception, and Calc3Exception share a common base class.

[Jacob Krall]

I had to implement "On Error Resume Next" in a language once, and the code I generated looked a lot like this.

[DeCaf]

Except of course for the fact that you should normally never catch Exception then. ;)

[DeCaf]

You could actually avoid the nested ifs by using && between each condition instead.

[Kiril]

@DeCaf as I said: that "should give you a general idea of how to do it (i.e. not an exact solution)." So the OP can catch whatever the appropriate exception happens to be... no need to catch the generic Exception.

[DeCaf]

Yeah, sorry, just felt the need to get that out there.

[Kiril]

@DeCaf, it's a valid clarification for those who might not be that familiar with the best practices. Thanks :)

[jjoelson]

This works nicely. There actually is a common base exception for the calcs (which I should have mentioned), so I don't even have to catch (Exception)

[TomTom]

On top, he assumes a common signature - which is not really that far off. Good answwer.

[jjoelson]

Also, I added continue in the catch block and break after the catch block so that the loop ends when a calculation works (thanks to Lirik for this bit)

[Bill K]

+1 only because it says "Don't use exceptions for control flow" although I would have used "NEVER EVER" instead of "As far as possible".

[user606723]

Honestly, I think this just causes unnecessary abstractions. This isn't a horrible solution, but I wouldn't use this for most cases.

[Jeff Ferland]

Please don't ever use a switch statement to create a for loop.

[Mohamed Abed]

If you don't care about the exception type, and you just want to handle conditional code .. thus it is definitely better in terms of abstraction and maintainability to convert it into a conditional method with return whether it is succeeded or not, this way you hide the exception handling messy syntax with a clear descriptive method .. then your code will handle it as it is a regular conditional method.

[user606723]

I know the points, and they are valid. However, when using this type of abstraction (hiding messyness/complexity) almost everywhere it becomes ridiculous and understanding a piece of software for what it is becomes much more difficult. As I said, it's not a terrible solution, but I wouldn't use it lightly.

[Mohamed Abed]

It is not maintainable to have switch statement for looping

[Adam Robinson]

@jjoelson: A break statement following calc(); within the try (and no continue statement at all) might be a little more idiomatic.

[user]

Doesn't that always call every function in the list? Might want to throw (pun not intended) in something like if(succeeded) { break; } post-catch.

[jjoelson]

I love this solution. However, it's fairly opaque to anyone who hasn't previously had exposure to monads, which means this is very far from idiomatic in c#. I wouldn't want one of my coworkers to have learn monads just to modify this one silly piece of code in the future. This is great for future reference, though.

[jp2code]

+1 orn. This is what I do. I only have to code one catch, the message sent to me (track in this case), and I know exactly what segment in my code caused the block to fail. Maybe you should have elaborated to tell members like DeCaf that the track message is sent to your custom error handling routine that enables you to debug your code. Sounds like he didn't understand your logic.

[Orn Kristjansson]

Well, @DeCaf is correct, my code segment does not keep executing the next function which is what jjoelson asked for, there for my solution is not feasable

[Nate C-K]

+1: This is the cleanest, most no-nonsense solution to the problem. The other solutions I see here are just trying to be cute, IMO. As the OP said, he didn't write the API so he's stuck with the exceptions that are thrown.

[Nate C-K]

+1 for sense of humor, for writing the most obtuse and verbose solution possible to this problem and then saying that it will "reduce the amount of boilerplate code you have to write while increasing the readability of your code".

[Kirk Broadhurst]

This is ok - similar to what OP suggested as far as wrapped the calculations. I'd just prefer something like while (!calcResult.HasValue) nextCalcResult(), instead of a list of Calc1, Calc2, Calc3 etc.

[musefan]

I know my answer is not the most efficient code, but then again using try/catch blocks for this kind of thing is not the best way to go anyway. Unfortunately, the OP is using a 3rd party library and has to do the best they can to ensure success. Ideally, the input could by validated first and the correct calculation function chosen to ensure it will not fail - of course, you could then put all that in a try/catch just to be safe ;)

[Sebastian Good]

Hey, we don't complain about the massive amounts of code lurking in System.Linq, and happily use those monads all day. I think @fre0n just mean that if you're willing to put the Maybe monad in your toolkit, these kinds of chained evaluations become easier to look at and reason about. There are several implemntations that are easy to grab.

[Artur Czajka]

return DoCalc(c++) is equivalent to return DoCalc(c) - post-incremented value won't be passed deeper. To make it work (and introduce more obscurity) it could be more like return DoCalc((c++,c)).

[Lenar Hoyt]

Is this pattern possible too in Java?

[Dax Fohl]

Just because it uses Maybe doesn't make this a monadic solution; it uses zero of the monadic properties of Maybe and so may as well just use null. Besides, used "monadicly" this would be the inverse of the Maybe. A real monadic solution would have to use a State monad that keeps the first non-exceptional value as its state, but that'd be overkill when normal chained evaluation works.

[AlexMelw]

I just want to say "Thank you". I tried to implement what you were talking about. I hope that I understood it correctly.

[Alex from Jitbit]

Yeah, sometimes the "design problem" is out of our reach. And sometimes it even comes from Microsoft. Take the FindTimeZoneById which is part .NET Core/Framework, it just fails with an exception if I'm passing a linux TimezoneID under Windows, then Windows timzoneId under linux, or Posix timezone under MacOS etc. have to try everything :(