Evil use of Maybe monad and extension methods in C#?

Question

edit 2015 This question and its answers are no longer relevant. It was asked before the advent of C# 6, which has the null propagating opertor (?.), which obviates the hacky-workarounds discussed in this question and subsequent answers. As of 2015, in C# you should now use Form.ActiveForm?.ActiveControl?.Name.

---

I've been thinking about the null propagation problem in .NET, which often leads to ugly, repeated code like this:

Attempt #1 usual code:

string activeControlName = null;
var activeForm = Form.ActiveForm;
if (activeForm != null)
{
    var activeControl = activeForm.ActiveControl;
    if(activeControl != null)
    {
        activeControlname = activeControl.Name;
    }
}

There have been a few discussions on StackOverflow about a Maybe<T> monad, or using some kind of "if not null" extension method:

Attempt #2, extension method:

// Usage:
var activeControlName = Form.ActiveForm
                          .IfNotNull(form => form.ActiveControl)
                          .IfNotNull(control => control.Name);

// Definition:
public static TReturn IfNotNull<TReturn, T>(T instance, Func<T, TReturn> getter)
    where T : class
{
    if (instance != null ) return getter(instance);
    return null;
}

I think this is better, however, there's a bit of syntactic messy-ness with the repeated "IfNotNull" and the lambdas. I'm now considering this design:

Attempt #3, Maybe<T> with extension method

// Usage:
var activeControlName = (from window in Form.ActiveForm.Maybe()
                         from control in window.ActiveControl.Maybe()
                         select control.Name).FirstOrDefault();

// Definition:
public struct Maybe<T> : IEnumerable<T>
      where T : class
{
    private readonly T instance;

    public Maybe(T instance)
    {
        this.instance = instance;
    }

    public T Value
    {
        get { return instance; }
    }

    public IEnumerator<T> GetEnumerator()
    {
        return Enumerable.Repeat(instance, instance == null ? 0 : 1).GetEnumerator();
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return this.GetEnumerator();
    }
}

public static class MaybeExtensions
{
    public static Maybe<T> Maybe<T>(this T instance)
        where T : class
    {
        return new Maybe<T>(instance);
    }
}

My question is: is this an evil abuse of extension methods? Is it better than the old usual null checks?

Solution 1

It's interesting that so many people independently pick the name IfNotNull, for this in C# - it must be the most sensible name possible! :)

Earliest one I've found on SO: Possible pitfalls of using this (extension method based) shorthand

My one (in ignorance of the above): Pipe forwards in C#

Another more recent example: How to check for nulls in a deep lambda expression?

There are a couple of reasons why the IfNotNull extension method may be unpopular.

1. Some people are adamant that an extension method should throw an exception if its this parameter is null. I disagree if the method name makes it clear.

2. Extensions that apply too broadly will tend to clutter up the auto-completion menu. This can be avoided by proper use of namespaces so they don't annoy people who don't want them, however.

I've played around with the IEnumerable approach also, just as an experiment to see how many things I could twist to fit the Linq keywords, but I think the end result is less readable than either the IfNotNull chaining or the raw imperative code.

I've ended up with a simple self-contained Maybe class with one static method (not an extension method) and that works very nicely for me. But then, I work with a small team, and my next most senior colleague is interested in functional programming and lambdas and so on, so he isn't put off by it.

Solution 2

Much as I'm a fan of extension methods, I don't think this is really helpful. You've still got the repetition of the expressions (in the monadic version), and it just means that you've got to explain Maybe to everyone. The added learning curve doesn't seem to have enough benefit in this case.

The IfNotNull version at least manages to avoid the repetition, but I think it's still just a bit too longwinded without actually being clearer.

Maybe one day we'll get a null-safe dereferencing operator...

---

Just as an aside, my favourite semi-evil extension method is:

public static void ThrowIfNull<T>(this T value, string name) where T : class
{
    if (value == null)
    {
        throw new ArgumentNullException(name);
    }
}

That lets you turn this:

void Foo(string x, string y)
{
    if (x == null)
    {
        throw new ArgumentNullException(nameof(x));
    }
    if (y == null)
    {
        throw new ArgumentNullException(nameof(y));
    }
    ...
}

into:

void Foo(string x, string y)
{
    x.ThrowIfNull(nameof(x));
    y.ThrowIfNull(nameof(y));
    ...
}

There's still the nasty repetition of the parameter name, but at least it's tidier. Of course, in .NET 4.0 I'd use Code Contracts, which is what I'm meant to be writing about right now... Stack Overflow is great work avoidance ;)

Solution 3

If you want an extension method to reduce the nested if's like you have, you might try something like this:

public static object GetProperty(this object o, Type t, string p)
{
    if (o != null)
    {
        PropertyInfo pi = t.GetProperty(p);
        if (pi != null)
        {
            return pi.GetValue(o, null);
        }
        return null;
    }
    return null;
}

so in your code you'd just do:

string activeControlName = (Form.ActiveForm as object)
    .GetProperty(typeof(Form),"ActiveControl")
    .GetProperty(typeof(Control),"Name");

I don't know if I'd want to use it to often due to the slowness of reflection, and I don't really think this much better than the alternative, but it should work, regardless of whether you hit a null along the way...

(Note: I might've gotten those types mixed up) :)

Solution 4

In case you're dealing with C# 6.0/VS 2015 and above, they now have a built-in solution for null propagation:

string ans = nullableString?.Length.ToString(); // null if nullableString == null, otherwise the number of characters as a string.

View more solutions

Comments

[Judah Gabriel Himango]

Well thanks for the feedback. Yeah, the ideal solution to this problem is some kind of null-safe dereference operator, e.g. someThing?.Foo? Unfortunately, the C# team doesn't seem too keen on adding something like this, so I don't think we'll see this in the near future.

[Daniel Earwicker]

It has the major disadvantage that if I change the name of the Name property to Description, then the code will still silently compile and then fail at runtime.

[Alex Black]

See my answer and Earwicker's links to previous answers, you can use a delegate (e.g. Func<A,B>) to get compile time safety to get the property you want instead of using string names of properties.

[Pedro d'Aquino]

Couldn't you use reflection to magically discover the parameter name?

[Jon Skeet]

@Pedro: Not really. Even if you could reliably work out the method that called the extension method (without worrying about inlining) you'd still just have a value. In some cases you could then use reflection, if it were unambiguous - but not very often.

[Mark Synowiec]

@Alex: Nice, hadn't thought about that, but if I ever decide it's worth using in code, I'll have to remember that. Thanks for the note :)

[Mark Synowiec]

@Earwicker: Very good point. That's another disadvantage to note.

[Dirk Vollmar]

And utilizing short-cut evaluation would mean that there is no need for a nested if: if (Form.ActiveForm != null && Form.ActiveForm.ActiveControl) fits in one line and is easily understood.

[Judah Gabriel Himango]

The ActiveForm thing was only an example -- usually the local variable is needed because it's an expensive function call, e.g. foo.GetExpensive(). With the initial sample, GetExpensive would need to be called multiple times. Yuck.

[Judah Gabriel Himango]

"in .NET 4 I'd use Code Contracts" Jon, the more I read about CC, the less impressed I am. A recent MSDN article seems to suggest CC is just for debugging purposes, as opposed to runtime enforcement. Thoughts?

[Judah Gabriel Himango]

I don't care for this solution. You're basically throwing a custom exception after each line of code. Doesn't feel any cleaner, doesn't give us much.

[danbst]

1. I see - it's no more usefull than just catching NullReferenceException. You are right here

[danbst]

2. Excepetion mechanism is somewhat more usefull, when your code will grow. You don't have to introduce lambda-style function call to C# (or equivalent LINQ-style) when only what you want is get activeControl.Name 3. Custom exc was introduced, 'cause of possible different type assertion, like "assumeIsTrue" and others. That had happend with my project.

[Alex M]

If you using ThrowIfNull in conjunction with FxCop, you could introduce [ValidatedNotNullAttribute] so it stops complaining on the CA1062. More here: esmithy.net/2011/03/15/suppressing-ca1062

[Jon Skeet]

@AlexM: Interesting. Fortunately I'm not an FxCop user, so I've never run into this myself...

[Judah Gabriel Himango]

Yep. This is clearly the way to go now that C# 6 is here. When I asked this question back in 2009 (!), the null propagating operator was merely a twinkle in Anders' eye. :-)

[MCattle]

As of C# 6.0, the ThrowIfNull() extension can now be made more elegant with the NameOf Expression: throw new ArgumentNullException(nameof(value)); which eliminates the need for the name parameter.

[Jon Skeet]

@MCattle: Indeed... and you can use a Roslyn code analyzer to confirm that you do so. Right now it's bed time though, so I'm not going to update the answer...