try catch inside a catch clause

Question

I'm currently loading images in different ways like this:

try {
   // way 1
} 
catch 
{ // way 1 didn't work
   try {
      // way 2
   } 
   catch 
   {
      // etc.
   }
}

I was wondering if there was a cleaner way to do this. Currently it's not a problem but if i add a few more ways it's going to get messy.
Note that the method loading the image is also in a try catch in the same way because it might not be an image.
It's basically trying a bunch of stuff to figure out what it is you dragged into the application.

`"why" didn't it work?` and handling that is a better design practice rather than letting something throw an error and hoping you cover it with nested catches (especially for something like this). What is happening that you are throwing errors trying to load the image? — Nyra, Oct 27 '14 at 16:55
you can have multiple catch statements instead. I would also advise to **prevent** the try failing instead of **curing** the problem in the first place — , Oct 27 '14 at 16:55
Do the libraries you use not provide a way to check if the data is valid first? In any case - I would probably extract out each 'attempt' into a separate method. For example: TryGetJPG() - which returns a stream (or whatever you'd like) on success, null on fail. — Rob, Oct 27 '14 at 16:55
http://stackoverflow.com/questions/7796420/pattern-to-avoid-nested-try-catch-blocks — Sybren, Oct 27 '14 at 16:55
I'm a firm believer in `catch` handlers should only contain code relevant to handling the exception, and nothing more. If you have a need for something like in your example, you should restructure the code. — helrich, Oct 27 '14 at 16:57
Agreed with @alykins. Is the reason for the first exception something that's actually *exceptional* or is it more like "this file doesn't start with the right signature so I'm going to try another method that might work"? — Bob Kaufman, Oct 27 '14 at 16:58
@InBetween I'm confused... Are you saying we should downvote or were there downvotes that went away (I don't think it deserves them personally, OP just seems to be newer coder)? — Nyra, Oct 27 '14 at 17:11
@alykins The answer has been downvoted once...this site is becoming a downvoting fest...the amount of questions that are being unfairly downvoted is IMO, to say the least, bewildering. I'm not sure what the reason is. — InBetween, Oct 27 '14 at 17:14
@InBetween preaching to the choir- I've already b$#'ed about it on meta lol; I agree but we're on the verge of derailing question o.o — Nyra, Oct 27 '14 at 17:19

score 9 · Accepted Answer · answered Oct 27 '14 at 16:56

You can write a method that accepts any number of delegates, attempting all of them and stopping after one of them runs successfully. This abstracts the exception handling into one place, and avoids all of the repetition:

public static void AttemptAll(params Action[] actions)
{
    var exceptions = new List<Exception>();
    foreach (var action in actions)
    {
        try
        {
            action();
            return;
        }
        catch (Exception e)
        {
            exceptions.Add(e);
        }
    }
    throw new AggregateException(exceptions);
}

This allows you to write:

AttemptAll(Attempt1, Attempt2, Attempt3);

If the methods compute a result you can create a second overload to handle that as well:

public static T AttemptAll<T>(params Func<T>[] actions)
{
    var exceptions = new List<Exception>();
    foreach (var action in actions)
    {
        try
        {
            return action();
        }
        catch (Exception e)
        {
            exceptions.Add(e);
        }
    }
    throw new AggregateException(exceptions);
}

+1 for aggregating and throwing the exceptions if all actions fail. — juharr, Oct 27 '14 at 17:45
I really like this solution. It's clean and clear. Also, it's what I was looking for just now. — Bovaz, Sep 02 '16 at 07:22

score 1 · Answer 2 · answered Oct 27 '14 at 16:58

Assuming the "different ways" to load the image all throw an exception upon failure, you could iterate through the different ways, until one succeeds. The example below uses a Function<Image> to show a parameterless function that returns an image upon success. In your concrete example, you probably also have parameters into your function.

List<Function<Image>> imageLoaders = LoadTheListSomehow();

foreach (var loader in imageLoaders)
{
    try
    {
        var image = loader();
        break;  // We successfully loaded the image
    }
    catch (Exception ex) 
    {
        // Log the exception if desired
    }
}

score 1 · Answer 3 · answered Oct 27 '14 at 16:59

The nesting there does look unnecessary. I would isolate each way of loading an image into its own private method, and then called these methods as delegates in a loop, like this:

private static MyImage LoadFirstWay(string name) {
    return ...
}
private static MyImage LoadSecondWay(string name) {
    return ...
}
private static MyImage LoadThirdWay(string name) {
    return ...
}
...
public MyImage LoadImage(string name) {
    Func<string,MyImage>[] waysToLoad = new Func<string,MyImage>[] {
        LoadFirstWay
    ,   LoadSecondWay
    ,   LoadThirdWay
    };
    foreach (var way in waysToLoad) {
        try {
            return way(name);
        } catch (Exception e) {
            Console.Error("Warning: loading of '{0}' failed, {1}", name, e.Message);
        }
    }
    return null;
}

try catch inside a catch clause

3 Answers3