views:

314

answers:

5

So, we are using the Enterprise Library 4.1 Exception Handling Application Block to deal with logging/handling our exceptions in a multiple-project application. We have a few custom exceptions and are throwing some exceptions whose classes are defined in the .NET framework's standard class libraries (e.g. ArgumentException, InvalidOperationException, ArgumentNullException, etc.).

Today, our team lead decided that he didn't want us to use the latter, since the .NET framework would throw those types of exceptions, and in order to facilitate filtering with the application block's policies, we should use only custom exceptions, going so far as to practically duplicate .NET standard class library exceptions with custom versions, as in CustomArgumentException, CustomInvalidOperationException, etc.

My question is, what is wrong with this approach? I couldn't put my finger on it at the time, but it smelled wrong to me and I haven't been able to shake my uneasy feelings about it. Am I worried about something that really isn't that big of a deal? I guess it just feels like the tail wagging the dog a bit here...

A: 

Well, Exceptions thrown by your code and thrown by the .net base classes should both be handled the same way.

Both are probably the symptom of a problem in your code, so neither should be ignored, or filtered !

Brann
+10  A: 

Ick. What I don't like about it is:

  • It duplicates existing types
  • It violates the principle of least surprise
  • It means that if you want to find everywhere you've used the wrong argument value (say) you have to look for two type hierarchies of exception instead of just looking for ArgumentException.

I would suggest you get your team lead to read item 60 of Effective Java 2nd edition. Yes, it's about Java rather than C# - but the principles remain the same.

Jon Skeet
LOL - "principle of least surprise"
Kev
Why is that laughable? See http://en.wikipedia.org/wiki/Principle_of_least_astonishment
Jon Skeet
Not laughable in a sarcastic way, just funny, I've never seen that before.
Kev
Ah right - fair enough :)
Jon Skeet
+3  A: 

The Framework Design Guidelines book (first edition) by Krzysztof Cwalina and Brad Abrams recommend throwing existing exceptions defined in the System namespaces where you can, and the more specific the better. If there isn't a good fit then throw a custom exception.

Creating a parallel universe of CustomArgumentNullException to match System.ArgumentNullException is a duplication of effort that I don't see any value in. At the end of the day if your code throws a System.ArgumentNullException rather than a framework class you can determine from the stack trace who was ultimately responsible.

This smells of unnecessary extra work for the present and in the future when it comes to code maintenance time.

HTH
Kev

Kev
A: 

Throwing .Net exceptions is fine when the exception rightly describes the type of problem you're trying to expose (e.g. when an argument is null you should throw a ArgumentNullException). If you find a situation that isn't handled by the .Net framework (e.g. You want to divide 6 by 3 but that isn't allowed by your application) you should create a custom exception.

Matt Ruwe
+3  A: 

I echo the answers by Jon Skeet and Kev. I'd just add that if your exception policy wants to treat framework-level exceptions different to your own exceptions, consider using the exception's stack trace.

// Somewhere within a custom exception handler policy
var frame = new StackTrace(exception).GetFrame(0);
if (frame.GetMethod().DeclaringType.Namespace.StartsWith("MyNamespace.")) 
{
    // Handle exceptions from our code
}
else 
{
    // Handle framework-level exceptions
}
Paul Stovell