Exception data evolved

As detailed in my earlier post about exception signatures I and my colleagues take exceptions from our production servers seriously. In addition to grouping them by signatures (which helps a lot and makes triage much more pleasant) and logging them in our internal bug tracking software we also try to add relevant debug information whenever we throw an exception.

The BCL team in their infinite wisdom added the Exception.Data property. This property is simply an IDictionary which allows storing key and value pairs of any type. By default this is an empty collection which means you don’t have to worry about it being null.

Typical usage

public void LogonUser(string username, string password, string domain)
{
    try
    {
        DataStore.LogOnUser(username, password, domain);
    }
    catch (Exception exc)
    {
        var ae = new ApplicationException("Underlying logon failed, see InnerException", exc);
                
        ae.Data["username"] = username;
        ae.Data["domain"] = domain;

        throw ae;
    }
}

When the exception reaches our internal trac site it might look something like the image below (in our case the data dictionary will actually be added as a comment but you get the point).

This has proven to be is incredibly useful for debugging purposes but it is a bit tedious to actually write the code. Storing the exception in a variable just to get access to the data property just didn’t feel right to me and writing the same boilerplate .Data[“xyz”] = xyz was just boring.

Step 1 – The AddData extension method.

I started off by creating an extension method for System.Exception called AddData. AddData looks a bit like this (very simplified, see the end of the post for the real deal)

public static Exception AddData(this Exception exception, string key, object value)
{
    exception.Data.Add(key, value);
    // whohoo, chaining!
    return exception;
}

This allowed me to save a few keystrokes over the first method and I would end up with something like this instead

catch(...) {
    throw new ApplicationException("Underlying logon failed, see InnerException", exc)
        .AddData("username", username)
        .AddData("domain", domain);
}

It might not look like much but it sure helps when you want to add debug data to an already existing exception, it saves you from having to store the reference and so on but it still doesn’t solve the problem with having to write the argument names twice. This isn’t only a nuisance when writing the code; it’s very easy for the key and value to get out of sync if you’re doing refactoring since refactoring tools will only change the variable name, not the key.

My ideal solution would be something like this

catch(...) {
    throw new ApplicationException("Underlying logon failed, see InnerException", exc)
        .AddData(username)
        .AddData(domain);
}

And have the AddData method automatically infer the proper key name but since that’s not possible I had to find another way.

Step 2 – Taking a cue from ASP.NET MVC

We continued using the AddData extension method above for a couple of weeks before it dawned on me that I could use anonymous types to skip the parameter name duplication. Anonymous types have the great ability of being able to “infer” property names when initialized.

var x = new { username = username, password = password }
// Is the same as
var x = new { username, password }

With this in mind I wrote an AddData extension method which accepts a single object and then uses reflection to iterate over all properties and adds their property names and values into the exception data dictionary.  This allowed me to rewrite my code yet again.

catch(...) {
    throw new ApplicationException("Underlying data store logon failed, see InnerException", exc)
        .AddData(new { username, password });
}

Neat, isn’t it? This is the exact same technique that ASP.NET MVC uses for route-declarations, attributes in html helpers and more.

Reflection? Isn’t that horribly slow

Not like in the olden days. It comes with a cost but you shouldn’t be too worried since you’re probably not throwing exception often enough for it to matter anyway (and if you are, then you have bigger problems).

Teh codez

using System;
using System.ComponentModel;
using System.Diagnostics;

namespace freakcode.Extensions
{
    /// 

    /// Extension methods related to instances of System.Exception and inherited objects.
    /// 
    public static class ExceptionExtensions
    {
        /// 

        /// Adds the supplied debug data to the exceptions data dictionary and returns
        /// the exception allowing chaining.
        /// 
        /// The exception type, you should not need to specify this explicitly
        /// The exception.
        /// The key of the debug value to be inserted into the exceptions data dictionary.
        /// The value to be inserted into the exceptions data dictionary.
        /// key is null
        /// An element with the same key already exists in the Data dictionary
        public static T AddData<T>(this T exception, string key, object value) where T : Exception
        {
            if (exception == null)
                throw new ArgumentNullException("exception");

            if (key == null)
                throw new ArgumentNullException("key");

            /* Key or value is not serializable (or key is null). The default internal structure which
             * implements the IDictionary is going to throw an exception in Add() so instead of 
             * throwing another exception while preparing to throw the first one we silently ignore the
             * error. Unless we're building in debug mode that is, then we'll fail. */
            if (value != null && !value.GetType().IsSerializable)
            {
                Debug.Fail("Attempt to add non-serializable value to exception data");
            }
            else
            {
                exception.Data.Add(key, value);
            }

            return exception;
        }

        /// 

        /// Adds the each property name and value from the supplied object to the exceptions data dictionary and returns
        /// the exception allowing chaining.
        /// 
        /// The exception type, you should not need to specify this explicitly
        /// The exception.
        /// An object from where properties will be read and added to the exception debug data collection.
        /// key is null
        /// An element with the same key already exists in the Data dictionary
        public static T AddData<T>(this T exception, object values) where T : Exception
        {
            if (values == null)
            {
                // Some really nasty things can happen if you start throwing exceptions in the middle
                // of throwing exceptions so unless we're in debug more we'll just silently ignore it.
                Debug.Fail("Argument 'values' was null!");
            }
            else
            {
                foreach (PropertyDescriptor descriptor in TypeDescriptor.GetProperties(values))
                    exception.AddData(descriptor.Name, descriptor.GetValue(values));
            }
            
            return exception;
        }
    }
}

Licensing information

ToTitleCase

Not to long ago I had to write a method to convert lowercase names to proper case for display on a website I was working on. I started rolling my own but then I thought that such a feature would be nice to have in the framework. So I hit google with a query and sure enough the framework authors had thought of it. The method was well hidden though ;)

CultureInfo.InvariantCulture.TextInfo.ToTitleCase("hELLo wORLd");
// Will return "Hello World"

This fits quite well in as an extension method

/// <summary>
/// Creates a new string with Title Case (ie "hEllO wORLd" becomes  "Hello World") using the Invariant Culture
/// </summary>
/// <param name="s">The string to convert</param>
/// <returns>The string in title case</returns>
public static string ToTitleCaseInvariant(this string s)
{
    return ToTitleCase(s, CultureInfo.InvariantCulture);
}

/// <summary>
/// Creates a new string with Title Case (ie "hEllO wORLd" becomes  "Hello World")
/// </summary>
/// <param name="s">The string to convert</param>
/// <returns>The string in title case</returns>
public static string ToTitleCase(this string s)
{
    return ToTitleCase(s, CultureInfo.CurrentCulture);
}

/// <summary>
/// Creates a new string with Title Case (ie "hEllO wORLd" becomes  "Hello World")
/// </summary>
/// <param name="s">The string to convert</param>
/// <param name="ci">The culture to use when creating title case</param>
/// <returns>The string in title case</returns>
public static string ToTitleCase(this string s, CultureInfo ci)
{
    if (s == null)
        throw new ArgumentNullException("s");

    return ci.TextInfo.ToTitleCase(s);
}

Now you can just call the ToTitleCase on your string objects like this:

var s = "george washington";
s.ToTitleCase();

Licensing information

The hazards of extension methods

I love extension methods! I really do, to the point that I have to constrain myself so that this blog won't turn into a shrine where followers of extension methods hang around all day praising the founders for giving us this precious gift.

That being said though I have one major issue with extension methods. Since exception methods is essentially glorified static methods they will accept you calling them even if the object reference is null.

string s = null;
s.ToString(); // This will throw a NullReferenceException
s.HtmlEncode(); // This will not

Now, did you see the difference? Since it's the string object we're talking about we all know that the HtmlEncode method doesn't belong there and thus we can deduce that it's an extension method but what if it where some lesser known object and the method name wasn't so obvious? It works since the s.HtmlEncode call will be compiled into something like this: StringExtensions.HtmlEncode(s)

Well, I thought about it for a while and I decided that I don't like it! I don't like it one bit actually and while I do respect the decision to make it this way I feel that it will essentially undermine the respect that C# developers have for null references. Whenever I look at some code calling an instance method there is something, embedded deep in my cerebral cortex, that tells me that I should watch out for nullity but when I see extension methods that allow (or actually depends on) the reference to be null my fear of NullReferenceException gradually goes away.

So what's my recommended solution? Well, with great power comes great responsibility and of course it's up to you whether or not you're going to include these methods in your code. I will not! Whenever I write an extension method I explicitly check for nullity and raise the ArgumentNullException.

The worst use of this this that I've seen so far is without a doubt the IsNull extension method which essentially lets you do this:

string s = null;

if(!s.IsNull())
    PerformWork(s)

if(s != null)
    PerformWork(s)

Now do you see the problem here? It's actually more code, less obvious, (IMO) less readable than the "original" null comparison and it breaks a very important rule; you can't call instance methods on null references. Now, I have no problem with the IsEmpty extension method since that doesn't break any null rules.

PS. This whole post is actually a reaction to the anonymous comment suggesting that I convert my Collection.IsNullOrEmpty method into an extension method. DS.

Licensing information

StartsWithAnyOf extension method

Here comes another great string extension for improved readability. You all know how to check if a string starts with another string (yep, it's the StartsWith method I'm talking about).

But what if you wan't to check if a string starts with any of a series of strings? Well, you'd have to do something like this.

string name = "Mr. Markus Olsson"
var l = new List<string> { "Dr", "Mr", "Ms" };
bool found;
foreach(string s in l) {
    if(name.StartsWith(l)) {
        found = true;
        break;
    }
}

Or, you could use lambdas for a much more elegant solution

string name = "Mr. Markus Olsson"
var l = new List<string> { "Dr", "Mr", "Ms" };
bool found = l.Exists(prefix => name.StartsWith(prefix));

That's pretty cool, right? The .Exists method on the list object takes a Predicate as a parameter and executes that predicate with each element as it's first argument until it finds a match. Coolio indeed but we can do better readability wise.

Enter StartsWithAnyOf extension methods

/// <summary>
/// Checks to see if the string starts with any of the supplied strings
/// </summary>
/// <param name="s">The string to check for a start value</param>
/// <param name="strings">One or more strings</param>
/// <returns>True the strings starts with any of the supplied strings, false otherwise</returns>
public static bool StartsWithAnyOf(this string s, params string[] strings)
{
    if (s == null)
        throw new ArgumentNullException("s");

    if (strings == null)
        throw new ArgumentNullException("strings");

    if (strings.Length == 0)
        throw new ArgumentOutOfRangeException("strings", "You must supply one or more strings");

    return Array.Exists(strings, (prefix => s.StartsWith(prefix)); 
}

/// <summary>
/// Checks to see if the string starts with any of the supplied strings
/// </summary>
/// <param name="s">The string to check for a start value</param>
/// <param name="strings">One or more strings</param>
/// <returns>True the strings starts with any of the supplied strings, false otherwise</returns>
public static bool StartsWithAnyOf(this string s, List<string> strings)
{
    if (s == null)
        throw new ArgumentNullException("s");

    if (strings == null)
        throw new ArgumentNullException("strings");

    if (strings.Count == 0)
        throw new ArgumentOutOfRangeException("strings", "You must supply one or more strings");

    return strings.Exists(x => s.StartsWith(x));
}

This allows us to rewrite our code to

string name = "Mr. Markus Olsson"
bool found = name.StartsWithAnyOf("Dr.", "Mr.", "Ms.");

Readability in a nutshell. Variations of these extension methods includes an override that takes a StringComparison in order to allow for case insensitive lookup. The EndsWithAnyOf method is of course also a must.

Update: as mattias pointed out there's a bit of code duplication here but that's intentional, read why

Update 2: I changed my mind again after discussing it with mattias and for the sake of readability and code-duplication I've decided to wrap the first method into a call of the second.

Update 3: James Curran pointed out that the Array class have an Exist method and that's of course what you want for the string array. Thanks James!

Licensing information