Search code examples
c#nesteddepthndepend

How to refactor to reduce nesting depth with try/multiple catches (NDepend)


In the following code, NDepend reports a violation because of a nesting depth of 6 (1 for each catch) and the limit is 5. So it gets flagged in this rule:

Quick summary of methods to refactor

Is having just a single catch for the more general IOException and having code within that catch to distinguish between DirectoryNotFound, PathTooLong, and other IOException the best way to go? I don't want to increase the nesting depth limit since it's a valid limit for most cases. What about the case where none of the exceptions are in the same hierarchy and combining isn't an option? Is creating an attribute and altering the rule to disable the violation for just this method the only way to go?

private static void _TryDeleteFile(string filename)
{
    try
    {
        File.Delete(filename);
    }
    catch (ArgumentException innerEx)
    {
         // do something
    }
    catch (DirectoryNotFoundException innerEx)
    {
        // do something
    }
    catch (PathTooLongException innerEx)
    {
        // do something
    }
    catch (IOException innerEx)
    {
        // do something
    }
    catch (NotSupportedException innerEx)
    {
        // do something
    }
    catch (UnauthorizedAccessException innerEx)
    {
        // do something
    }
}

Solution

  • Firstly try/catch blocks are not as elegant to work with in c# as they could be, and that's just something we have to live with for now.

    Secondly, there are many ways to reduce nesting but they will always come at the expense of some other form of complexity, so you need to be careful. Remember that NDepend is recommending that you reduce nesting because it is guessing that your code might be hard to read or maintain. It could be wrong, or it could be that there are no better options given the tools you have available.

    In my mind, the way forward really depends on the work you are doing inside each of those catch blocks. If it is very simple, such as just returning a string error message and nothing else, then I think your code will be fine as is. However if you are doing different types of logic in each block, and particularly if you have repeated logic for different exception types, then your code may become messy.

    The refactoring might be to keep a collection of 'exception handlers' (either classes or Actions) in a dictionary, factory class or IOC kernel. You can then source the exception handler in one line of code. This will reduce nesting, but introduce its own complexity. Something like this:

    try
    {
       /* Your code */
    }
    catch(Exception ex)
    {
        var exceptionHandler = _exceptionHandlers[ex.GetType()];
        exceptionHandler.Execute(ex);
    }
    

    There are many different ways of achieving essentially the same thing: you push each blob of logic out into it's own class/method/lambda and then you have an intermediate class/method/Dictionary that you use for sourcing the correct logic.