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?
String.Concat
. That, too, can throw an exception.Throwable
, specifically, anOutOfMemoryError
.StringBuilder
to build strings that I almost thought to suggest using that instead ofString.Concat()
, but if you are throwing enough of these exceptions where that actually makes a difference then there are much bigger problems.... Your corrected version of the exception is what I would expect to see.