What's the point of try...catch..rethrow in constructors?

Nov 20, 2007 at 4:38 PM
Edited Nov 20, 2007 at 4:38 PM
All classes I could see on a quick pass have the following structure in the constructors:

        public SyndicationEndpoint(Uri source, string mimeType)
        {
            //------------------------------------------------------------
            //	Attempt to initialize class state
            //------------------------------------------------------------
            try
            {
                //------------------------------------------------------------
                //	Validate parameters
                //------------------------------------------------------------
                if (source == null)
                {
                    throw new ArgumentNullException("source");
                }
                if (String.IsNullOrEmpty(mimeType))
                {
                    throw new ArgumentNullException("mimeType");
                }
 
                //------------------------------------------------------------
                //	Set class members
                //------------------------------------------------------------
                endpointMediaType   = mimeType.Trim();
                endpointSource      = source;
            }
            catch
            {
                //------------------------------------------------------------
                //	Rethrow exception
                //------------------------------------------------------------
                throw;
            }
        }

What's the point of catching just to rethrow? (and document it also :S). This creates an unnecessary catch region for no reason. Removing it would behave exactly the same and would make for much cleaner code.

Maybe I'm missing something, but I've NEVER seen .NET code (or guidelines for that matter) that use such an (anti)pattern... Global try...catch blocks are generally considered a bad thing too...
Nov 20, 2007 at 4:47 PM
This is the most weird one:

        public ResponseHeader()
        {
            //------------------------------------------------------------
            //	Attempt to initialize class state
            //------------------------------------------------------------
            try
            {
                //------------------------------------------------------------
                //	
                //------------------------------------------------------------
                
            }
            catch
            {
                //------------------------------------------------------------
                //	Rethrow exception
                //------------------------------------------------------------
                throw;
            }
        }
Nov 20, 2007 at 6:21 PM
Edited Nov 20, 2007 at 6:22 PM
Daniel,

What you are seeing is a defect in my personality as well as an artifact in how I set debugging breakpoints. I use breakpoints on the last global catch to catch when an unexpected exception was thrown. The code is actually wrong, in that I typically would have used the following, with the XML comment documentation would indicate that an ArgumentNullException can be expected by the caller.

public SyndicationEndpoint(Uri source, string mimeType)
{
    //------------------------------------------------------------
    //	Attempt to initialize class state
    //------------------------------------------------------------
    try
    {
        //------------------------------------------------------------
        //	Validate parameters
        //------------------------------------------------------------
        if (source == null)
        {
            throw new ArgumentNullException("source");
        }
        if (String.IsNullOrEmpty(mimeType))
        {
            throw new ArgumentNullException("mimeType");
        }
 
        //------------------------------------------------------------
        //	Set class members
        //------------------------------------------------------------
        endpointMediaType   = mimeType.Trim();
        endpointSource      = source;
    }
    catch (ArgumentNullException)
    {
        //------------------------------------------------------------
        //	Rethrow argument null exception
        //------------------------------------------------------------
        throw;
    }
    catch
    {
        //------------------------------------------------------------
        //	Rethrow exception
        //------------------------------------------------------------
        throw;
    }
}

I agree that this could be rewritten as follows:

public SyndicationEndpoint(Uri source, string mimeType)
{
    //------------------------------------------------------------
    //	Validate parameters
    //------------------------------------------------------------
    if (source == null)
    {
        throw new ArgumentNullException("source");
    }
    if (String.IsNullOrEmpty(mimeType))
    {
        throw new ArgumentNullException("mimeType");
    }
 
    //------------------------------------------------------------
    //	Set class members
    //------------------------------------------------------------
    endpointMediaType   = mimeType.Trim();
    endpointSource      = source;
}

I tend to be very verbose in my commenting and try-catch blocks, but did not realize this would have a major impact on performance, other than the stack traces tend to be larger. If you feel this is causing a major performance impact on the framework, I would love to learn more about how, and I will create a work item to make the appropriate changes.

In the future I will try to refrain from being so quirky, but you can just chalk this up to skinning a cat in more than one way I guess.
Nov 20, 2007 at 10:39 PM
Got it!

What I find useful in debugging exceptions is to check the CLR Exceptions | Thrown checkbox in the Debug -> Exceptions menu. From there you can move up the call stack to the ultimate "catcher" if there's any, but typically I want to stop execution at the throwing point...

Thanks!