Don’t Break My Mental Model – Beware of C# Extension Methods

Last updated on 2019-09-22

I think extension methods is one of the things I love most in C# and is definitely missing in Java. For those that are not familiar with extension methods, they let you add methods to existing classes. You do this by creating a static method that receives the class you want to extend as the first parameters with the this keyword. It can be done for any class you can reference, even external libraries, even primitive types. Here’s an example of an extension method for a string:

public static class MyStringExtensions
{
    public static string ToCrazyCase(this string theString)
    {
        string result = "";
        for (int i = 0; i < theString.Length; i++)
            result += (i % 2 == 0 ? theString[i].ToString().ToUpper() : theString[i].ToString().ToLower());
        return result;
    }
}

var x = "hello".ToCrazyCase() // x is now "HeLlO";

Pretty cool, right?

Yes, it’s cool, but only if you respect the rules of the game. Notice that a string reference is passed to the method, and this reference can be null, but it is up to you to decide what to do when this happens. In my case, the behavior is as expected – if the method is called with a null string, the method will throw a NullReferenceException.

Now let’s say one of your friendly co-workers decided to write the method like this:

public static string ToCrazyCase(this string theString)
{
    if (theString == null || theString.Length == 0)
        return "";

    string result = "";
    for (int i = 0; i < theString.Length; i++)
        result += (i % 2 == 0 ? theString[i].ToString().ToUpper() : theString[i].ToString().ToLower());
    return result;
}

Nothing wrong with adding some null checks and returning a default value, right?

Wrong. Because it messes up our mental model on how null values are handled by the language. Let me explain what I mean by that.

In some part of a big codebase you are writing a function that uses ToCrazyCase, oblivious to how it handles null. Something like this:

public string MyStringManipulator(string s)
{
    // Do all kinds of stuff
    var z = s.ToCrazyCase();
    // Do more stuff that manipulates z
    return z;
}

When your code is running, MyStringManipulator starts returning empty strings… Which is weird because if s is null the assumption is that s.ToCrazyCase() will throw a NullReferenceException. But is doesn’t. And this here destroys a basic assumption that you have from the language: calling a method on a null reference will always throw an exception.

And now every time you use an extension method you must take this into account. And your mental model of the language is now broken and will take some time fixing.

There are ways to at least reduce the pain here, such as naming the method ToCrazyCaseNullSafe, but IMHO in this case there should be two functions (one that is null safe and one that isn’t). And it should also be used sparingly (I wrote a while ago about forcing the use of async, and my view hasn’t changed – don’t force me to change my mental model).

We have created a mental model of how the language works, and this mental model makes us productive. Please, don’t break this model.