On two projects I have worked on, I’ve seen a lot of this:

public void DoSomething()
        // actually do something here
    catch (Exception ex)
        throw new Exception("Unable to do something", ex);

I’ve seen this in various differing styles:

  • not including the original exception in the newly thrown one (ouch!)* having a custom Exception type, indicating where in the code/assemblies the exception happened.Reasons seem to be a desire to easily see where the exception occurred. So a DomainException would indicate the exception happened in the Domain DLL. Another reason was to easily see the methods called. In the log of an exception, you can then see all the “sub-exceptions”.

Of course, this can all be seen in the stacktrace, and I believe it has its origins in the old days of coding, where you would have to set up your exception handling manually. But in .NET, this all happens automatically, and you’ve got a full stacktrace to help you find the problem.

Even more, wrapping each method in a try-catch construction gives for more code-noise when you’re scanning a class. More for your brains to process, harder to find what you’re looking for, etc.

So: no advantages, only disadvantages. Only catch an exception if you’ll do something with/about it. Log it, swallow it, mention it to the user. But if you’re only going to rethrow it, don’t. Just don’t. Catch your exceptions at the top-most level.

Oh, and never do this:

if (myObject == null)
    throw new Excpetion("myObject is null");

Yes, I’ve seen this. It’s no shame if you’ve ever coded this, but remove it when you see it.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes:

<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>

This site uses Akismet to reduce spam. Learn how your comment data is processed.