Subject009
Subject009

Reputation: 79

try catch inside a catch clause

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.

Upvotes: 1

Views: 143

Answers (3)

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726669

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;
}

Upvotes: 1

Eric J.
Eric J.

Reputation: 150118

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

Upvotes: 1

Servy
Servy

Reputation: 203835

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);
}

Upvotes: 9

Related Questions