Throwing an exception from within a constructor of an exception

Question

As I was TDD-ing some code, it occurred to me that one of my typed exceptions could potentially throw a NullReferenceException while creating its error message.

Here's the constructor for the typed exception in question:

public class MyTypedException : Exception
{
  public MyTypedException(String expectedValue, ICollection<String> possibleValues)
  {
    String message = String.Format("Failed to find {0}.  Possible values:", expectedValue);
    foreach(var value in possibleValues)
    {
      message = String.Concat(message, ", ", value);
    }
    /* Assigns message to property, etc... */
  }
}

If possibleValues is null, then the constructor of MyTypedException will throw a NullReferenceException. My first reaction was to think "Well, just check if possibleValues is null, and if it is, throw an ArgumentNullException. However, throwing a different exception from the constructor of an excpetion just seems...wrong.

The alternative would be to check whether possibleValues is null or empty, and if it is, create a different message, like so:

public MyTypedException(String expectedValue, ICollection<String> possibleValues)
{
  String message = String.Empty;
  if (possibleValues == null || possibleValues.Count <= 0)
    message = "Failed to find {0} because the collection of possibles values is null or empty";
  else
  {
    message = String.Format("Failed to find {0}.  Possible values:", expectedValue);
    foreach (var value in possibleValues)
    {
      message = String.Concat(message, ", ", value);
    }
  }
  /* Assigns message to property, etc... */
}

Is it ever okay to throw a different exception when attempting to create an exception?

Answer 1

IMHO, it is not OK.

When you throw a MyTypedException, the most important aspect is that this error happened. Secondary errors or usage bugs might be of interest of course, but they are secondary and masking the original one is a really bad idea I think.

Of course, one cannot avoid all errors (esp. like the mentioned OutOfMemory or similar), but barring fatal errors (like OOM, etc.) knowingly throwing something other than the intended Exception doesn't seem like a good idea to me.

Do note: In your example, if ArgumentNullException indicates a bug in the calling code, then other means of catching the bug are of course a Good Thing:

• You can Assert and then
• just continue in the production code with the above workaround
• additionally do some warning/debug logging
• and/or FailFast if you think this bug is critical. (I wouldn't.)

Also note that delaying any formatting of the message to later might also make sense (just keep a ref to the passed arguments, and format on request.)

Not (manually) formatting at all, hence avoiding the whole problem of the example might even be better.