What are good ways of balancing informative exceptions and clean code?

Question

With our public SDK, we tend to want to give very informative messages about why an exception occurs. For example:

if (interfaceInstance == null)
{
     string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    throw new InvalidOperationException(errMsg);
}

However, this tends to clutter up the flow of the code, as it tends to put a lot of focus on error messages rather than what the code is doing.

A colleague started refactoring some of the exception throwing to something like this:

if (interfaceInstance == null)
    throw EmptyConstructor();

...

private Exception EmptyConstructor()
{
    string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    return new InvalidOperationException(errMsg);
}

Which makes the code logic easier to understand, but adds a lot of extra methods to do error handling.

What are other ways to avoid the "long-exception-messages clutter logic" problem? I'm primarily asking about idiomatic C#/.NET, but how other languages manage it are helpful as well.

[Edit]

It would be nice to have the pros and cons of each approach as well.

Answer 1

Why not have specialized exception classes?

if (interfaceInstance == null)
{
    throw new ThisParticularEmptyConstructorException(<maybe a couple parameters>);
}

That pushes the formatting and details to the exception itself, and leaves the main class uncluttered.

Answer 2

Microsoft seems to (via looking at the .NET source) sometimes use resource/Environment strings. For example, ParseDecimal:

throw new OverflowException(Environment.GetResourceString("Overflow_Decimal"));

Pros:

• Centralizing exceptions messages, allowing for re-use
• Keeping the exception message (arguably which don't matter to code) away from the logic of the methods
• The type of exception being thrown is clear
• Messages can be localized

Cons:

• If one exception message is changed, they all change
• The exception message is not as easily available to the code throwing the exception.
• Message is static and contains no information about what values are wrong. If you want to format it, it's more clutter in the code.

Answer 3

For the public SDK scenario, I would strongly consider using Microsoft Code Contracts as these provide informative errors, static checks and you can also generate documentation to add into XML docs and Sandcastle generated help files. It is supported in all paid for versions of Visual Studio.

An additional advantage is that if your customers are using C#, they can leverage your code contract reference assemblies to detect potential problems even before they run their code.

The full documentation for Code Contracts is here.

Answer 4

The technique I use is to combine, and outsource the validation and throwing altogether to a utility function.

The single most important benefit is that it is reduced down to a one-liner in the business logic.

I bet you can't do better unless you can reduce it further - to eliminate all argument validations and object-state guards from the business logic, keeping only the operational exceptional conditions.

There are, of course, ways to do that - Strongly typed language, "no invalid objects allowed anytime" design, Design by Contract, etc.

Example:

internal static class ValidationUtil
{
    internal static void ThrowIfRectNullOrInvalid(int imageWidth, int imageHeight, Rect rect)
    {
        if (rect == null)
        {
            throw new ArgumentNullException("rect");
        }
        if (rect.Right > imageWidth || rect.Bottom > imageHeight || MoonPhase.Now == MoonPhase.Invisible)
        {
            throw new ArgumentException(
                message: "This is uselessly informative",
                paramName: "rect");
        }
    }
}

public class Thing
{
    public void DoSomething(Rect rect)
    {
        ValidationUtil.ThrowIfRectNullOrInvalid(_imageWidth, _imageHeight, rect);
        // rest of your code
    }
}

Answer 5

[note] I copied this from the question into an answer in case there are comments about it.

Move each exception into a method of the class, taking in any arguments that need the formatting.

private Exception EmptyConstructor()
{
    string errMsg = string.Format(
          "Construction of Action Argument: {0}, via the empty constructor worked, but type: {1} could not be cast to type {2}.",
          ParameterInfo.Name,
          ParameterInfo.ParameterType,
          typeof(IParameter)
    );

    return new InvalidOperationException(errMsg);
}

Enclose all of the exception methods into region, and place them at the end of the class.

Pros:

• Keeps the message out of the core logic of the method
• Allows for adding logic information to each message (you can pass in arguments to the method)

Cons:

• Method clutter. Potentially you could have a lot of methods that just return exceptions and aren't really related to the business logic.
• Cannot reuse messages in other classes

Answer 6

If you can get away with a little more general errors you could write a public static generic cast function for you, that infers the source type:

public static I CastOrThrow<I,T>(T t, string source)
{
    if (t is I)
        return (I)t;

    string errMsg = string.Format(
          "Failed to complete {0}, because type: {1} could not be cast to type {2}.",
          source,
          typeof(T),
          typeof(I)
        );

    throw new InvalidOperationException(errMsg);
}


/// and then:

var interfaceInstance = SdkHelper.CastTo<IParameter>(passedObject, "Action constructor");

There are variations possible (think SdkHelper.RequireNotNull()), that only check requirements on the inputs, and throw if they fail, but in this example combining the cast with producing the result is selfdocumenting and compact.

If you are on .net 4.5, there are ways to have the compiler insert the name of the current method/file as a method parameter (see CallerMemberAttibute). But for an SDK, you probably can't require your customers to switch to 4.5.