The Problem(s) with Nullable Reference Types
Tags: csharp, dotnet, codequality
Published: 2019-11-02 14:47:07

---
title: The Problem(s) with Nullable Reference Types
published: true
tags: csharp, dotnet, codequality
cover_image: https://thepracticaldev.s3.amazonaws.com/i/xqt5r7q4b2vz2r9tsg6w.png
---

There are already [some](https://dev.to/integerman/safer-code-with-c-8-non-null-reference-types-4f2c) [posts](https://dev.to/jeikabu/net-core-30-preview-w-c-80-nullable-reference-types-5e53) about C# 8's nullable reference types. Adopting this new feature has been great fun! And almost all existing code can be adapted to use this feature to improve it's safety.

Almost all code...

# Initializers

Using initializers is somewhat common. In this example it's used to avoid duplicate code in multiple constructors, but there are other use cases:

- To split complex initialization code into nicely maintainable smaller methods
- To avoid throwing exceptions from a constructor
- To have `async` initialization logic

Consider this class:

```csharp
class ClassWithInit
{
    public object SomeProperty { get; set; }
    
    public ClassWithInit() { // ⚠ Warning: Non-nullable property 'SomeProperty' is uninitialized.
        Init();
    }
    
    public ClassWithInit(string someArg) {
        Init();
    }
    
    // This class has multiple constructors,
    //  initialization logic is reused by placing it in an 'Init' method.
    private void Init() {
        SomeProperty = ...;
    }
}
```
Even though you call `Init` from the constructor and are therefore certain the property will be initialized the compiler doesn't realize this. To get rid of the warning you can trick the compiler:
```csharp
public object SomeProperty { get; set; } = null!;
```
This looks a bit odd and if you ever rewrite the `Init` method and forget to initialize the property you won't be warned!

# Events

A minor gripe I have is having to mark all my events as nullable:

```csharp
event PropertyChangedEventHandler? PropertyChanged;
```
Event handlers are of course objects, but because of the `+=`, `-=` syntax we hardly think of them as such and, for me at least, raising events with `?.Invoke(...)` is basically second nature.

Marking eventhandlers as nullable is a logical consequence of the new syntax and we'll just have to get use to it. But it can be a bit of a shock when you set nullable to enabled on an existing project and are confronted with hundreds of warnings on completely harmless code.

# Generics

While the new `?` syntax *looks* similar to existing `System.Nullable<T>` syntax for structs, it is completely different in practice. You mainly notice this when working with generic classes, especially when you **don't** specify a `where T : class/struct` constraint.

Take for example this `RelayCommand<T>` class ([DelegateCommand](https://prismlibrary.github.io/docs/commanding.html) in Prism), a common construction in WPF projects:

```csharp
public class RelayCommand<T> : ICommand
{
    public Action<T> Body { get; }

    public void Execute(object? param)
        => Body((T)param); // ⚠ Warning: Possible null reference assignment

    public RelayCommand(Action<T> body)
    {
        Body = body;
    }
    // 'CanExecute' functionality omitted for brevity
}
```

The parameter `T` could be anything: `struct` *or* `class`, this makes this class more generally useful. Moreover the parameter `param` of the `Execute` method *should* be marked nullable as it is probably called from a XAML binding to `CommandParameter` and who knows if that'll be null. (Even a `RelayCommand<int>` could be called with `null` as the parameter since `Execute(...)` simply takes an `object` as parameter)

In a perfect world we'd write:

```csharp
public Action<T?> Body { get; } // Error
```
That would fix the warning now produced by the `Execute` method and it would helpfully inform users of this class that they must provide a delegate capable of handling `null` values to the constructor. Alas, this is not possible, changing the property type to `Action<T?>` results in an error:

> **CS8627:** A nullable type parameter must be known to be a value type or non-nullable reference type. Consider adding a 'class', 'struct' or type constraint.

But keeping the code as it is could lead to dangerous situations. Consider:

```csharp
// Dangerous code, but no warning:
new RelayCommand<string>(s => Console.Write(s.Length));

// Safe code:
new RelayCommand<string?>(s => Console.Write(s != null ? s.Length : -1));
```
Because `string` '`s`' could be null the first piece of code mights still lead to the dreaded `NullReferenceException`. The proper code specifies the type as `string?` but there is no warning to remind you to do so.

The solution, sadly 😢, is to create two versions of this class, one for reference types, and one for structs.

If we add the constraint `where T : class` to our `RelayCommand<T>` class we can now use type `Action<T?>` as 'Body'. After this change passing a method to the constructor that forgets to check for null **will** result in a warning: Huzzah, we've done it 🎉. This results in safer code, exactly what this new feature is for. Shame it means having write two versions of a class which before this feature was perfectly happy handling both structs and classes.

Or is there another way?

### Attributes

In *some* cases the issues with generics which accept both structs and classes can be mitigated using C# 8's new attributes:

```csharp
// Event args for an event which is raised when some property of type 'T' is changed,
//  works for both structs and classes thanks to attributes
public class ItemChangedEventArgs<T> : EventArgs
{
    [MaybeNull]
    public T OldValue { get; }
    [MaybeNull]
    public T NewValue { get; }

    public ItemChangedEventArgs([AllowNull]T oldValue, [AllowNull]T newValue)
    {
        OldValue = oldValue;
        NewValue = newValue;
    }
}
```
	
This class behaves exactly as you'd hope it would, for example:

- `new ItemChangedEventArgs<string>(null, "")` doesn't produce a warning even though null is passed and `T` is not marked `T?`.
- `((ItemChangedEventArgs<object>)var1).NewValue.ToString()` produces a warning because object is a reference type and 'NewValue' could be null.
-  `((ItemChangedEventArgs<int>)var2).NewValue.ToString()` doesn't produce a warning because the compiler realises an `int` is never going to be null.

There is also the [notnull constraint](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/generics/constraints-on-type-parameters#notnull-constraint) which mitigates some other generic-related issues, but this post is getting too long...

---

# Conclusion

Generally C# 8's nullable reference types is a neat feature which is sure to improve code quality and reduce bugs. And even if there are still minor exception where you *do* have to worry about null, we can be grateful nothing is `undefined` at least 😉.

**Share your thoughts!** Have you run into any problems related to this new feature? Or do you have some brilliant way to handle the issues I've laid out? Be sure to leave a comment.