Refactoring exception handling

Question

In one of my classes I have a call to a repository which has some error handling on it. I would like to refactor the error handling code because it is quite repetitive and the only thing that really changes is the message.

My code currently looks something like this:

public IList<User> GetUser()
{
    try
    {
        return _repository.GetUsers();
    }
    catch (WebException ex)
    {
        ErrorMessages.Add("...");
        _logger.ErrorException("...", ex);
    }
    catch (SoapException ex)
    {
       ErrorMessages.Add("...");
        _logger.ErrorException("...", ex);
    }
    ... etc
}

I could replace those lines in my catch block with a call to another method which takes an error message value and a logger message value. However I suppose I could also do this using an Action<> parameter but I am very inexperienced at using Func<> and Action<> and don't really see what benefit I would have using one of those over a method.

My question is really what is the best way to refactor this code and why does one way benefit over the other (as per my example above).

Thanks for any help.

Answer 1

You can use lambdas to help with this.

If you define your general-purpose error handler to accept a parameter of type Action then you can call that action in the error handler.

You don't need to worry about return values because the lambda you write at the point of call can take care of that.

For example, your general handler could look like this:

public void AttemptAction(Action action)
{
    try
    {
        action();
    }
    catch (WebException ex)
    {
        ErrorMessages.Add("...");
        _logger.ErrorException("...", ex);
        // Rethrow?
    }
    catch (SoapException ex)
    {
        ErrorMessages.Add("...");
        _logger.ErrorException("...", ex);
        // Rethrow?
    }
}

And then you could use it like this:

public IList<User> GetUser()
{
    IList<User> result = null;

    AttemptAction(() => result = _repository.GetUsers());

    return result;
}

Answer 2

Assuming the exception types are always the same but the messages are different, you can do this:

static public T Try<T>(string webMessage, string soapMessage, Func<T> func)
{
    try
    {
        return func();
    }
    catch (WebException ex)
    {
        ErrorMessages.Add(webMessage);
        _logger.ErrorException(webMessage, ex);
    }
    catch (SoapException ex)
    {
       ErrorMessages.Add(soapMessage);
        _logger.ErrorException(soapMessage, ex);
    }
}

This Try-method will use a delegate of type Func<T> to call a function and return its value. The function will be inside the same try-catch block. The messages are provides via parameters. Now, somewhere else in your code, you could call this like:

var users = Try("My web message.", "My soap message.", () => _repository.GetUsers());

Or, in your case even shorter (when not using parameters):

var users = Try("My web message.", "My soap message.", _repository.GetUsers);

Of course you can modify and arrange the parameters of Try to your own liking.

In case you are mixing method with and without return types, it is better not to use the Func but the Action. This will be able to comply to all situations:

static public void Try(string webMessage, string soapMessage, Action action)
{
    try
    {
        action();
    }
    catch (WebException ex)
    {
        ErrorMessages.Add(webMessage);
        _logger.ErrorException(webMessage, ex);
    }
    catch (SoapException ex)
    {
       ErrorMessages.Add(soapMessage);
        _logger.ErrorException(soapMessage, ex);
    }
}

But this solution makes the code a tiny bit more difficult to read / maintain:

IList<User> users;
Try("My web message.", "My soap message.", () => users = _repository.GetUsers());