1

I'm running FXCop to clean up my code, and FXCop complains about how Im catching the exception. "You should not catch Exception or SystemException. Catching generic exception types can hide run-time problems from the library user, and can complicate debugging. You should catch only those exceptions that you can handle gracefully."

Here is a sample of my code, does anyone know how this can be improved, (so that FxCop does not complain?)

Thanks :)

catch(Exception e) { if(e is IOException) { Console.WriteLine("{0} System IO Exception", e); } if (e is DirectoryNotFoundException) { Console.WriteLine("{0} Directory not found", e); } if (e is ArgumentException) { Console.WriteLine("{0} Directory is invalid", e); } if(e is PathTooLongException) { Console.WriteLine("{0} Directory path is too long", e); } if (e is UnauthorizedAccessException) { Console.WriteLine("{0} Unauthorised to delete directory", e); } } 
3
  • 1
    It might be useful to specify in your question what hints FxCop is providing you. Commented May 25, 2011 at 14:54
  • 1
    It probably wants you to catch the specific exceptions directly, rather than casting a generic Exception to a more specific type once caught. Commented May 25, 2011 at 14:56
  • The reason the code above should be avoided is because if any exception other than the ones listed is thrown, it won't get propogated up the chain. Commented Sep 17, 2012 at 19:02

8 Answers 8

7

Multiple catches one for each type.

} catch(DirectoryNotFoundException ex){ Console.WriteLine("{0} Directory not found", ex); } catch(PathTooLongException exx){ Console.WriteLine("{0} Directory path is too long", exx); } catch(IOException e){ Console.WriteLine("{0} System IO Exception", e); } catch(ArgumentException e){ Console.WriteLine("{0} Directory is invalid", e); } catch(UnauthorizedAccessException exxx){ Console.WriteLine("{0} Unauthorised to delete directory", exxx); } catch(Exception exxxx){ Console.WriteLine("{0} plain old exception", exxxx); } 

A Note About Order

Keep in mind that you want to put child exception classes (in terms of inheritance) earlier in your list.

Example: You have ChildException which inherits from ParentException. In the catch block you want to list ChildException prior to ParentException. If ParentException is listed first then any ChildException thrown will be caught in the first branch it can be up-cast to.

EDIT

  • fixed order to account for inheritance
Sign up to request clarification or add additional context in comments.

4 Comments

Thanks, by "most specific to least specific", does it mean I should go from "DirectoryNotFoundException" to "IOException" (since DirectoryNotFoundException is more specific than IOException ) and so on?
I think they mean in terms of inheritance... This example is actually a bad one since DirectoryNotFoundException and PathTooLongException actually both inherit from IOException: msdn.microsoft.com/en-us/library/system.io.ioexception.aspx You'd want to catch DirectoryNotFoundException first, because it would never fall through if you caught IOException first.
@DanielSchaffer - thanks for pointing that out -- i hadn't looked up the inheritance order for those exceptions -- updated post to reflect.
It's also worth noting that you can use the same parameter name for the exception in all the catch statements since they're all scoped separately.
4

You can catch specific exceptions and have multiple catch blocks, you don't need to do the if statements.

I'm guessing it doesn't like the fact that you're catching just Exception and not the specific exception.

catch(IOException e) { Console.WriteLine("{0} System IO Exception", e); } catch(ArgumentException e) { Console.WriteLine("{0} Directory is invalid", e); } 

Note: You can't catch the DirectoryNotFoundException after the IOException, because IOException would have already caught it.

2 Comments

Thanks, I realized it :) I guess if I put them in reverse order, it should work :)
@Rosie, yep. If you put DirectoryNotFound above IOExcpetion, it should work fine.
2

For one, I would use if...else if...else not continuous ifs.

Secondly, the catch block can do this as-is.

try { ... } catch (DirectoryNotFoundException dnfex) { // class DirectoryNotFoundException : IOException Console.WriteLine("{0} Directory Not Found", dnfex); } catch (IOException ioex) { // class IOException : SystemException Console.WriteLine("{0} System IO Exception", ioex); } catch (SystemException sex) { // class SystemException : Exception Console.WriteLine("{0} System Exception", sex); } catch (Exception ex) { Console.WriteLine("{0} Generic Exception", ex); } 

The try...catch...finally syntax allows for multiple/different exceptions to be caught, just remember that it's not like a case statement. That is to say, once it has qualified one of the exception blocks it will not continue on to a more broad exception (if one exists) below [no fall-through mechanism].

EDIT updated my example to show how this should work. You want to start with the highest class first, and work your way down the inheritance tree. Basically, don't start with IOException first then DirectoryNotFoundException because a DirectoryNotFoundException is an IOException.

2 Comments

Thanks very much. I guess finally catches all the other exceptions then? Should I throw it, if that is the case?
@Rosie: finally is used when you need to clean up after a catch, not for catching exceptions. Basically, the try block runs as far as it can. If it succeeds (no exception) the finally is then processed. If it fails (an exception is thrown) the try block ends and the catch takes over, then it moves on to the finally. this is mostly helpful when you're opening connections, dealing with files, or something that should be cleaned up after it either succeeds or fails. See try-catch-finally (C#) on msdn for more info.
1
catch (IOException e) { } catch (DirectoryNotFoundException e) { } catch (ArgumentException e) { } 

and so on...

Or, if all you're doing is printing a message, you can do it with a single catch clause:

catch (Exception e) { Console.WriteLine(e.Message); } 

Comments

1

In lieu of more information from FxCop, the way you handle multiple types of exception is a bit non-standard.

C# already provides a mechanism for treating different exceptions in a different way - perhaps this will satisfy your FxCop master:

catch(IOException e) { Console.WriteLine("{0} System IO Exception", e); } catch(ArgumentException e) { Console.WriteLine("{0} Directory is invalid", e); } //.. etc 

Comments

1

You can catch each exception like so:

catch(IOException e) { Console.WriteLine("{0} System IO Exception", e); } catch(DirectoryNotFoundException e) { Console.WriteLine("{0} Directory not found", e); } etc... 

which will get around it. Also this will let an uncaught exception bubble up. As you have it now ALL exceptions are caught and just ignored unless they are in your list.

2 Comments

Thanks, in this case, should I throw the uncaught exception?
@Rosie Sure. If it can't handle the exception, someone higher up might. you can do so by putting a "throw" on a line by itself inside a catch block.
1

Do:

catch(IOExceptione e){ Console.WriteLine("{0} System IO Exception", e); } catch(DirectoryNotFoundException e){ Console.WriteLine("{0} Directory not found", e); catch(ArgumentException e){ Console.WriteLine("{0} Directory is invalid", e); } ... 

And only if you're still worried you didn't catch an exception, do

catch(Exception e){ Console.WriteLine("{0} Directory is invalid", e); } 

After all other catches.

Comments

1

ideal way of catching exception is this you should not check conditions in catch if enum of catch is available

catch(IOException ioex) { Console.WriteLine("{0} System IO Exception", e); } catch (DirectoryNotFoundException dx) { Console.WriteLine("{0} Directory not found", e); } 

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.