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.

Answer 7

What we like to do for business logic errors (not neccessarily argument errors etc.) is to have a single enum that defines all potential types of errors:

/// <summary>
/// This enum is used to identify each business rule uniquely.
/// </summary>
public enum BusinessRuleId {

    /// <summary>
    /// Indicates that a valid body weight value of a patient is missing for dose calculation.
    /// </summary>
    [Display(Name = @"DoseCalculation_PatientBodyWeightMissing")]
    PatientBodyWeightMissingForDoseCalculation = 1,

    /// <summary>
    /// Indicates that a valid body height value of a patient is missing for dose calculation.
    /// </summary>
    [Display(Name = @"DoseCalculation_PatientBodyHeightMissing")]
    PatientBodyHeightMissingForDoseCalculation = 2,

    // ...
}

The [Display(Name = "...")] attributes define the key in the resource files to be used to translate the error messages.

Also, this file can be used as a starting point to find all occurences where a certain type of error is generated in your code.

Checking of Business Rules can be delegated to specialized Validator classes that yield lists of violated Business Rules.

We then use a custom Exception type to transport the violated rules:

[Serializable]
public class BusinessLogicException : Exception {

    /// <summary>
    /// The Business Rule that was violated.
    /// </summary>
    public BusinessRuleId ViolatedBusinessRule { get; set; }

    /// <summary>
    /// Optional: additional parameters to be used to during generation of the error message.
    /// </summary>
    public string[] MessageParameters { get; set; }

    /// <summary>
    /// This exception indicates that a Business Rule has been violated. 
    /// </summary>
    public BusinessLogicException(BusinessRuleId violatedBusinessRule, params string[] messageParameters) {
        ViolatedBusinessRule = violatedBusinessRule;
        MessageParameters = messageParameters;
    }
}

Backend service calls are wrapped in generic error handling code, which translates the violated Busines Rule into an user readable error message:

public object TryExecuteServiceAction(Action a) {
    try {
        return a();
    }
    catch (BusinessLogicException bex) {
        _logger.Error(GenerateErrorMessage(bex));
    }
}

public string GenerateErrorMessage(BusinessLogicException bex) {
    var translatedError = bex.ViolatedBusinessRule.ToTranslatedString();
    if (bex.MessageParameters != null) {
        translatedError = string.Format(translatedError, bex.MessageParameters);
    }
    return translatedError;
}

Here ToTranslatedString() is an extension method for enum that can read resource keys from [Display] attributes and use the ResourceManager to translate these keys. The value for the respective resource key can contain placeholders for string.Format, which match the provided MessageParameters. Example of an entry in the resx file:

<data name="DoseCalculation_PatientBodyWeightMissing" xml:space="preserve">
    <value>The dose can not be calculated because the body weight observation for patient {0} is missing or not up to date.</value>
    <comment>{0} ... Patient name</comment>
</data>

Example usage:

throw new BusinessLogicException(BusinessRuleId.PatientBodyWeightMissingForDoseCalculation, patient.Name);

With this approach, you can decouple the generation of the error message from the generation of the error, without the need to introduce a new exception class for each new type of error. Useful if different frontends should display different messages, if the displayed message should depend on the user language and/or role etc.

Answer 8

I would use guard clauses as in this example so parameter checks can be reused.

In addition, you can use the builder pattern so more than one validation can be chained. It will look a little bit like fluent assertions.