Redeveloping WordTutor
Nullable types redux

Continuing our the upgrade process from last time, in this post we’ll explore the changes required to our WordTutor.Desktop project.

The Null Object Pattern

In RoutedCommandSink, we get a warning in our constructors when we try to assign null to _canExecute. We could make this a nullable member, but there’s a better approach: by always assigning a value, we can simplify other code.

Initialize _canExecute with a simple lambda expression that always returns true:

public RoutedCommandSink(
    RoutedCommand command, Action execute)
    : base(command)
{
    _execute = execute 
        ?? throw new ArgumentNullException(nameof(execute));
    _canExecute = () => true;
}

We can now modify CanExecuteCore() by removing the guard clause:

protected override bool CanExecuteCore(object parameter)
{
    return parameter is T actual && _canExecute(actual);
}

Allowing Null Fields

By contrast, we’ll just change _commandHost into a nullable value:

private readonly INotifyPropertyChanged? _commandHost;

Caller Member Name

In our class ViewModelBase, we use the [CallerMemberName] attribute to declare our event trigger methods. This is useful as it helps to guarantee that the name passed to the method (and used within) correctly identifies the caller. Elimination of magic strings is almost always a good thing.

With nullable reference types, we run into a problem.

For [CallerMemberName] to work, the property must be declared with a default null value - but when we consume the value, we know that it will always be present.

One approach is to add a useless guard clause, throwing an exception if the value is missing (which will never happen).

protected void UpdateProperty<T>(...)
    where T : IEquatable<T>
{
    if (property is null)
    {
        throw new ArgumentNullException(nameof(property));
    }
    // ...
}

Another, better, approach is to apply the “dammit” operator (!) to assert that the value won’t be null. This useful operator allows us to instruct the compiler that we know better than it does, and that this value is safe. (Observe that the operator does not instruct the compiler that the value is non-null … if it did, the construct null! would be a compiler error.)

OnPropertyChanged(property!);

Propagation of Nullability

Subscriptions to our ReduxStore need to support null values, so any of the handlers we use when calling Subscribe() needs to have nullability set appropriately.

In VocabularyBrowserViewModel, our constructor has a warning on this statement:

_screenSubscription = _store.SubscribeToReference(
    app => app.CurrentScreen as VocabularyBrowserScreen,
    RefreshFromScreen);

While the first parameter returns a VocabularyBrowserScreen? (nullable), the method RefreshFromScreen only accepts VocabularyBrowserScreen (non-nullable).

To fix this, we need to modify the declaration of RefreshFromScreen to accept a nullable value:

private void RefreshFromScreen(
    VocabularyBrowserScreen? screen) { ... }

I’ve found this a few times - that the nullability modification required to fix a bug can be somewhere quite different to the location where the compiler emits a warning.

A bug in the factory

In the Create() method of ViewFactory, we’re getting a warning about a possible NullReferenceException:

public ContentControl Create(ViewModelBase viewModel)
{
    // elided
    var viewModelType = viewModel.GetType();
    if (!_mapping.TryGetValue(viewModelType, out var viewType))
    {
        viewType = FindViewType(viewModelType);
        _mapping[viewModelType] = viewType;
    }

    if (viewModelType == null)
    {
        // Warning shown here
        throw new InvalidOperationException(
            $"Failed to find ViewModel class for model type '{viewModelType.Name}'.");
    }

Can you see the bug?

The intent is to throw an exception if we failed to find a view type to match the provided view model - but we’re instead checking the view model type. When the bug is fixed, the warning goes away.

if (viewType == null)
{
    // Warning shown here
    throw new InvalidOperationException(
        $"Failed to find ViewModel class for "
        + "model type '{viewModelType.Name}'.");
}

I’m enjoying the way that nullable reference type checking by the compiler is revealing subtle bugs in my code.

Consistency of Nullability

When subscribing to our redux store, the constructor for AddVocabularyWordViewModel showed this warning:

CS8631: The type WordTutor.Core.AddVocabularyWordScreen? cannot be used as type parameter ‘V’ in the generic type or method IReduxStore<WordTutorApplication> .Subscribe<V>(Func<WordTutorApplication, V>, Action<V>). Nullability of type argument WordTutor.Core.AddVocabularyWordScreen? doesn’t match constraint type System.IEquatable<WordTutor.Core.AddVocabularyWordScreen?>.

Phew!

While I understood what the error was saying, it wasn’t at all clear what change was needed to make the warning go away. Worse, these C# features are so new that there’s almost nothing published online to read.

After (literally!) several hours of reading, thinking, and experimenting, I posted online to see if anyone else had run into the issue. In short order, someone came back with a solution.

The Subscribe() method has this declaration:

IDisposable Subscribe<V>(
    Func<T, V> reader,
    Action<V> whenChanged)
    where V : IEquatable<V>;

One cause of the CS8631 error is the generic type constraint on V. It makes sense to require that the nullability of V and its type constraints must agree. If, for example, V was nullable and the constraint was not, you would end up with the compiler failing to warn you about some potential null handling errors.

There’s another problem - the delegates Func<T, V> reader and Action<V> whenChanged are declared as non-nullable, preventing the use of this subscription with nullable values. We already have code the uses null values, so we need to change this.

We end up with this modified declaration:

IDisposable Subscribe<V>(
    Func<T, V?> reader,
    Action<V?> whenChanged)
    where V : class, IEquatable<V>?;

At first, I didn’t understand why this required the class constraint to be added, but after reading “Try out Nullable Reference Types” (specifically the section “The issue with T?”), I do (somewhat!) see the problem they’re avoiding. But, I’m not sure I can explain it yet in my own words. Go read all the gory details; it’s stuff you’re going to want to know.

Since we can’t do method overloading based solely on different type constraints, we need to split the original method into two: one for reference types, and one for value types.

IDisposable SubscribeToReference<V>(
    Func<T, V?> referenceReader,
    Action<V?> whenChanged)
    where V : class, IEquatable<V>?;

IDisposable SubscribeToValue<V>(
    Func<T, V> valueReader,
    Action<V> whenChanged)
    where V : struct, IEquatable<V>;

You’ll note above that I said “One cause of the CS8631 error” … there’s another way that it can occur. The warning can appear if nullability differs between a type constraint and the type itself. I originally had this declaration for VocabularyWord:

public class VocabularyWord 
    : IEquatable<VocabularyWord>

However, this conflicts with a type constraint T : IEquatable<T?> because the two differ in the nullability of the type constraint. One uses VocabularyWord (not nullable) and the other uses T? (nullable). I had to modify the declaration to this:

public class VocabularyWord 
    : IEquatable<VocabularyWord?>

After thinking this through, I came to the conclusion that this made sense - after all, it’s valid to ask if a word equals null, even if the answer is always going to be “no”.

Conclusion

Migration to use the nullability features of C# 8 will involve a bit of head-scratching, due in no small part to the need to reason through what null means for your code. It’s worth it though - the resulting code will be clearer, and you’ll almost certainly find and eliminate some subtle bugs.