C# How to handle multiple exceptions which does all the same?

Question

In my code I have a method with multiple catch statements, which perform all the same statement. I'm not sure this is the correct way to implement this. How would you do this?

public void LoadControl(ControlDestination controlDestination, string filename, object parameter)
{
    try
    {
        // Get filename with extension
        string file = GetControlFileName(filename);

        // Check file exists
        if (!File.Exists(file))
            throw new FileNotFoundException();

        // Load control from file
        Control control = LoadControl(filename);

        // Check control extends BaseForm
        if (control is BaseForm)
        {
            // Set current application on user control
            ((BaseForm)control).CurrentApplication = this;
            ((BaseForm)control).Parameter = parameter;

            // Set web user control id
            control.ID = filename;

            Panel currentPanel = null;

            switch (controlDestination)
            {
                case ControlDestination.Base:
                    // Set current panel to Base Content
                    currentPanel = pnlBaseContent;
                    // Set control in viewstate
                    this.BaseControl = filename;
                    break;
                case ControlDestination.Menu:
                    // Set current panel to Menu Content
                    currentPanel = pnlMenuContent;
                    // Set control in ViewState
                    this.MenuBaseControl = filename;
                    break;
            }

            currentPanel.Controls.Clear();
            currentPanel.Controls.Add(control);
            UpdateMenuBasePanel();
            UpdateBasePanel();

        }
        else
        {
            throw new IncorrectInheritanceException();
        }
    }
    catch (FileNotFoundException e)
    {
        HandleException(e);
    }
    catch (ArgumentNullException e)
    {
        HandleException(e);
    }
    catch (HttpException e)
    {
        HandleException(e);
    }
    catch (IncorrectInheritanceException e)
    {
        HandleException(e);
    }

}

This is how HandleException looks like:

private void HandleException(Exception exception)
{
    // Load error control which shows big red cross
    LoadControl(ControlDestination.Menu, "~/Controls/Error.ascx", null);

    // Store error in database
    DHS.Core.DhsLogDatabase.WriteError(exception.ToString());

    // Show error in errorbox on master
    Master.ShowAjaxError(this, new CommandEventArgs("ajaxError", exception.ToString()));
}

Answer 1

You are doing it right (you should catch only the exceptions you are going to handle and there is no way to catch more than one exception type in one single catch block), but as an alternative, you can just catch(Exception ex), check the exception type, and if it is not one that you expect just throw it again, something like this:

var exceptionTypes=new Type[] {
    typeof(FileNotFoundException),
    typeof(ArgumentNullException),
    //...add other types here
};

catch(Exception ex) {
    if(exceptionTypes.Contains(ex.GetType()) {
        HandleException(ex);
    } else {
        throw;
    }
}

Answer 2

Write it like this:

try
{
  // code that throws all sorts of exceptions
}
catch(Exception e)
{
  HandleException(e);
}

edit: note that this is a direct answer to your question, not a comment on whether or not this is a recommended practice.

edit2: you can however test in your function if the type of e is a specific list of exceptions and if it's not, you can rethrow it. Exception handling performance is a non-issue since it's meant to be... exceptional in the first place.

Answer 3

I'd refactor as follows:-

public class Sample
{
    public void LoadControl( ControlDestination controlDestination, string filename, object parameter )
    {
        HandleExceptions( HandleException, () =>
        {
            //.... your code
        } );
    }

    private void HandleExceptions( Action<Exception> handler, Action code )
    {
        try
        {
            code();
        }
        catch ( FileNotFoundException e )
        {
            handler( e );
        }
        catch ( ArgumentNullException e )
        {
            handler( e );
        }
        catch ( HttpException e )
        {
            handler( e );
        }
        catch ( IncorrectInheritanceException e )
        {
            handler( e );
        }
    }

    private void HandleException( Exception exception )
    {
        // ....
    }
}

If I was using VB.NET, I'd use exception filters to do series of catches. But as we're using C#, the approach you have is the most efficient one possible rather than doing

private void HandleExceptions( Action<Exception> handler, Action code )
    {
        try
        {
            code();
        }
        catch ( Exception e )
        {
            if ( e is FileNotFoundException
                || e is ArgumentNullException
                || e is HttpException
                || e is IncorrectInheritanceException )
                handler( e );
            else
                throw;
        }
    }

Answer 4

I'm going to answer this is in a language-agnostic manner:

1. What you have done now is correct. Nothing wrong with it, except that it might get tedious if you do it many times.

2. Catch the most general form of exception that there is. Simply

catch(Exception e)
{
    ...
}

3. Maybe you want to only catch some exceptions, without catching all exceptions, which is what you would be doing if you just did #2.

Do what you did in #2, plus modify HandleException to only handle certain types of exceptions. This way you will only ever have to do type tem out once, and it is still more compact than above.

private void HandleException(Exception e) throws Excpetion
{
    // Reject some types of exceptions
    if (!((e is FileNotFoundException) ||
        (e is ArgumentNullException) ||
        (e is HttpException ) ||
        (e is IncorrectInheritanceException )))
    {
        throw;
    }

    //Rest of code
    ...
}

Edit:

I see Konamiman has an improved version of this third option. I say go for that.

Answer 5

I would do it like this

public void LoadControl(ControlDestination controlDestination, string filename, object parameter)
{
    try
    {
        // Get filename with extension
        string file = GetControlFileName(filename);

        // Check file exists
        if (!File.Exists(file))
            throw new FileNotFoundException();

        // Load control from file
        Control control = LoadControl(filename);

        // Check control extends BaseForm
        if (control is BaseForm)
        {
            // Set current application on user control
            ((BaseForm)control).CurrentApplication = this;
            ((BaseForm)control).Parameter = parameter;

            // Set web user control id
            control.ID = filename;

            Panel currentPanel = null;

            switch (controlDestination)
            {
                case ControlDestination.Base:
                    // Set current panel to Base Content
                    currentPanel = pnlBaseContent;
                    // Set control in viewstate
                    this.BaseControl = filename;
                    break;
                case ControlDestination.Menu:
                    // Set current panel to Menu Content
                    currentPanel = pnlMenuContent;
                    // Set control in ViewState
                    this.MenuBaseControl = filename;
                    break;
            }

            currentPanel.Controls.Clear();
            currentPanel.Controls.Add(control);
            UpdateMenuBasePanel();
            UpdateBasePanel();

        }
        else
        {
            throw new IncorrectInheritanceException();
        }
    }
    catch (Exception e)
    {
        HandleException(e);
    }
}


public void HandleException(Exception e)
{
    if (e is FileNotFoundException
            || e is ArgumentNullException
            || e is HttpException
            || e is IncorrectInheritanceException)
    {
        // Load error control which shows big red cross
        LoadControl(ControlDestination.Menu, "~/Controls/Error.ascx", null);

        // Store error in database
        DHS.Core.DhsLogDatabase.WriteError(exception.ToString());

        // Show error in errorbox on master
        Master.ShowAjaxError(this, new CommandEventArgs("ajaxError", exception.ToString()));
    }
}

Answer 6

You can use generics for a much nicer solution as long as you don't mind using Lambda's too. I am not a fan of switching on type. I have used this code a few times, I find it comes in especially handy for service proxies in which you want to handle a number of exceptions in the same way. As has been stated above its always best to catch the right type of exception where possible.

The code works by specifying the exceptions as generic type arguments to the handle function. These specific types are then caught but passed to a generic handler as the base class. I didn't add a HandleAndThrow but this can be added as desired. Also change naming to your liking.

    public static void Handle<T>(Action action, Action<T> handler)
        where T : Exception
    {
        try
        {
            action();
        }
        catch (T exception)
        {
            handler(exception);
        }
    }

    public static void Handle<T1, T2>(Action action, Action<Exception> handler)
        where T1 : Exception
        where T2 : Exception
    {
        try
        {
            action();
        }
        catch (T1 exception)
        {
            handler(exception);
        }
        catch (T2 exception)
        {
            handler(exception);
        }
    }

    public static void Handle<T1, T2, T3>(Action action, Action<Exception> handler)
        where T1 : Exception
        where T2 : Exception
        where T3 : Exception
    {
        try
        {
            action();
        }
        catch (T1 exception)
        {
            handler(exception);
        }
        catch (T2 exception)
        {
            handler(exception);
        }
        catch (T3 exception)
        {
            handler(exception);
        }
    }

    public static void Handle<T1, T2, T3, T4>(Action action, Action<Exception> handler)
        where T1 : Exception
        where T2 : Exception
        where T3 : Exception
        where T4 : Exception
    {
        try
        {
            action();
        }
        catch (T1 exception)
        {
            handler(exception);
        }
        catch (T2 exception)
        {
            handler(exception);
        }
        catch (T3 exception)
        {
            handler(exception);
        }
        catch (T4 exception)
        {
            handler(exception);
        }
    }
}

public class Example
{
    public void LoadControl()
    {
        Exceptions.Handle<FileNotFoundException, ArgumentNullException, NullReferenceException>(() => LoadControlCore(10), GenericExceptionHandler);   
    }

    private void LoadControlCore(int myArguments)
    {
        //execute method as normal
    }

    public void GenericExceptionHandler(Exception e)
    {
        //do something
        Debug.WriteLine(e.Message);
    }        
}