Skip to content

Instantly share code, notes, and snippets.

@richlander
Last active May 2, 2023 11:36
Show Gist options
  • Star 100 You must be signed in to star a gist
  • Fork 7 You must be signed in to fork a gist
  • Save richlander/e3c0031e226ee06481668867955b82de to your computer and use it in GitHub Desktop.
Save richlander/e3c0031e226ee06481668867955b82de to your computer and use it in GitHub Desktop.
Modernizing a codebase for C# 9

Modernizing a codebase for C# 9

There are lots of cases that you can improve. The examples use nullable reference types, but only the WhenNotNull example requires it.

Use the property pattern to replace IsNullorEmpty

Consider adopting the new property pattern, wherever you use IsNullOrEmpty.

string? hello = "hello world";
hello = null;

// Old approach
if (!string.IsNullOrEmpty(hello))
{
    Console.WriteLine($"{hello} has {hello.Length} letters.");
}

// New approach, with a property pattern
if (hello is { Length: >0 })
{
    Console.WriteLine($"{hello} has {hello.Length} letters.");
}

You can use a similar super-powered set of checks on arrays. Note that the "Old approach" isn't compatible with nullability, but the "New approach" is. It is due to the compiler only tracking variables not array indices.

// For arrays
string?[]? greetings = new string[2];
greetings[0] = "Hello world";
greetings = null;

// Old approach
if (greetings != null && !string.IsNullOrEmpty(greetings[0]))
{
    Console.WriteLine($"{greetings[0]} has {greetings[0].Length} letters.");
}

// New approach
if (greetings?[0] is {Length: > 0} hi)
{
    Console.WriteLine($"{hi} has {hi.Length} letters.");
}

Here is some related code experiments on nullability and arrays: https://gist.github.com/richlander/ca6567039906da4e1fcfba557b6ccb63

Simplify checks to multiple constant values

You can now test a value against multiple constant values.

ConsoleKeyInfo userInput = Console.ReadKey();

// Old approach
if (userInput.KeyChar == 'Y' || userInput.KeyChar == 'y')
{
    Console.WriteLine("Do something.");
}

// New approach with a logical pattern
if (userInput.KeyChar is 'Y' or 'y')
{
    Console.WriteLine("Do something.");
}

Use NotNullWhen for bool return methods with nullable out parameters

You can make it easy to call methods that return bool and whose signature include an out param with a nullable annotation, using the NotNullWhen attribute. In the typical pattern, the attribute tells the compiler that the out parameter is set when the return value is true. In that case, you don't have to check the out parameter for a null reference, if you guard the use of the reference within an if statement, conditional on the return value.

You can search a codebase for this pattern with the following regex, to find opportunities to use NotNullWhen.

bool.*(out).*(\?)

I used VS Code for this, as demonstrated below.

image

It will match methods like the following:

public bool ListenToCardIso14443TypeB(TransmitterRadioFrequencyConfiguration transmitter, ReceiverRadioFrequencyConfiguration receiver, out Data106kbpsTypeB? card, int timeoutPollingMilliseconds)

You can update the method with a NotNullWhen attribute, like the following:

public bool ListenToCardIso14443TypeB(TransmitterRadioFrequencyConfiguration transmitter, ReceiverRadioFrequencyConfiguration receiver, [NotNullWhen(true)] out Data106kbpsTypeB? card, int timeoutPollingMilliseconds)

The NotNullWhen attribute enables consuming code to skip checking for null for out param, even though it is annotated as nullable. Example: https://github.com/dotnet/iot/blob/54469318f33124e3455bf974e6a75167dfb831e6/src/devices/Pn5180/Pn5180.cs#L1138-L1142

You can then write this consuming code: https://github.com/dotnet/iot/blob/54469318f33124e3455bf974e6a75167dfb831e6/src/devices/Pn5180/samples/Program.cs#L168

You can only use this attribute if you target .NET Core 3.0+. The #if in Pn5180.cs example is only necessary if you also target .NET Core 2.1 or earlier. The same pattern applies to .NET Standard 2.1 and pre-2.1.

Use MemberNotNull for member fields and properties that are set in helper methods called from a constructor

A common pattern is setting a member fields and properties from helper methods that are called from an object constructor. This is useful if there is a lot of work to do (and you prefer clean constructors), or if you want to share logic across multiple constructors. Both approaches are sensible, but do not play nicely with nullability. The compiler cannot see that the member field or property is reliably set. The solution to this (.NET 5.0+) is the apply to the MemberNotNull or MemberNotNullWhen attribute on the helper method that assigns a non-null value to one or multiple member fields or properties. As a result, they don't have to be (necessarily) set to nullable, which is a nice thing to avoid.

Docs:

Example usage: https://github.com/dotnet/iot/blob/54469318f33124e3455bf974e6a75167dfb831e6/src/devices/Bmxx80/Bmxx80Base.cs#L312

That example code targets both .NET Core 2.1 and .NET 5.0. That's why it using conditional compilation (#if) which isn't otherwise needed. This is what the code does in absense of that attribute being available, for .NET Core 2.1 and 3.1: https://github.com/dotnet/iot/blob/54469318f33124e3455bf974e6a75167dfb831e6/src/devices/Bmxx80/Bmxx80Base.cs#L90-L95.

Avoid ! (dammit operator), but do use for Dispose

I try to color within the lines as much as possible with nullable reference types. That means avoiding the use of the ! or "dammit" operator. I have found that it is better to to prefer nullable and the ? operator over '!'. Every time you using !, you are giving up on compiler checking. Why not just accept nulls, but with help from the compiler?

The one (very large) exception to this approach is dispose. For the sake of everything holy and virtuous, don't make a member nullable only to satisfy the requirements of dispose. You should feel free to assign null! to non-nbullable object members as part of dispose. All bets are off after dispose, so don't worry about the state of the object after that.

@thewayibyte
Copy link

Tell me why? Why the f** do such things for this pretty language? :(

@daneb
Copy link

daneb commented Nov 5, 2020

if (userInput.KeyChar is 'Y' or 'y')

OK, that's cool.

// New approach, with a property pattern
if (hello is { Length: >0 })

Eh, that's a step backwards in readability.

I like it. I'm reading it in my head as "if hello is an object whose length is greater than zero".

Perhaps this is more clearer? Doesn't quite feel so concise then.
hello has a Length property of 0 and if that property is greater than 0

@JudahGabriel
Copy link

JudahGabriel commented Nov 5, 2020

If I wanted to see if hello has a length of 5, I can just do this with C# 8 today:

if (hello?.Length == 5) {
}

Clearer and still abides in the C language style.

Don't get me wrong, pattern matching is awesome, and C# 9's additions to pattern matching are fantastic! But your string length example isn't the best use for it.

@programmation
Copy link

If I wanted to see if hello has a length of 5, I can just do this with C# 8 today:

if (hello?.Length == 5) {
}

Clearer and still abides in the C language style.

Don't get me wrong, pattern matching is awesome, and C# 9's additions to pattern matching are fantastic! But your string length example isn't the best use for it.

How do you test for length == 5 and / or some other value of a different property on the same object? Pattern matching makes this easy, and reduces repetition.

I like the fact that C# syntax is evolving, but still backwards-compatible.

I also like the fact that the new additions allow me to express meaning without having to say exactly how that meaning is derived.

Under the hood the compiler probably emits code much like the old style, but it does it correctly and can optimise what it's created. If I write old-style code, that may or may not be possible, and the potential for mistakes is much greater. Leaving off the ? by accident changes the meaning completely in the old style and opens up the possibility of NREs. There's no such mistake to make with the pattern-matching style, and NREs are automatically excluded.

@DanNeely
Copy link

DanNeely commented Nov 6, 2020

What i currently do for multiple value checking is generally:

var yes = new List { 'y', 'Y' };
if (yes.Contains(userInput.KeyChar)

Although in this particular case I'd probably do:

if (userInput.KeyChar.ToLower() =='y')

The is notation looks nice for shorter sets of values because it removes the need for a temp array/collection; but I'd still probably use it for testing more than 2 or 3 values because it remains less repetitious and easier to read.

@richlander
Copy link
Author

@DanNeely -- Your idea is a fine one. I suspect that the pattern approach will be faster. It's just two (possibly only one) check and no method calls.

@paulomorgado
Copy link

var yes = new List { 'y', 'Y' };
if (yes.Contains(userInput.KeyChar)

This is a performance killer in so many ways:

  • If there are more than 4 items, the list will need to grow.
    • If the size is know, always create collections with initial capacity.
  • The list will need to be scanned to find the match.
    • Use HashSet instead.

@DanNeely
Copy link

DanNeely commented Nov 8, 2020

var yes = new List { 'y', 'Y' };
if (yes.Contains(userInput.KeyChar)

This is a performance killer in so many ways:

  • If there are more than 4 items, the list will need to grow.

    • If the size is know, always create collections with initial capacity.
  • The list will need to be scanned to find the match.

    • Use HashSet instead.

99.9% of the time any performance limit in code I write is going to either be database related or due to an inefficient top level algorithm. Anywhere else in my code I'm focussing on readability (with a strong emphasis on reduced verbosity) and reduced chance of inadvertent bugs; long chains of conditionals are a miserable failure for both; which is why I normally use the collection.contains pattern.

The compiler not being smart enough to use an appropriate initial collection size for statically initialized collections is the sort of micro-optimization so far below the relevance threshold for anything I'm doing as to be irrelevant. Feel free to file a bug if it bothers you, setting an appropriate size for initial creation to prevent growing it should be an easy enhancement; even if confirming it's never expanded and can be set to an exact size has too many complex cases to be worthwhile.

As for hash collections, I might try measuring relative performance if I'm bored; but for the small collection sizes I normally use am skeptical that the overhead of doing the hashing would end up being less than that of doing the handful of comparisons avoided. And looping back to my original point, a single "why did you use a hashset here" conversation with a coworker would probably end up consuming more time than replacing it would save. Never mind that a even a short conversation would cost as much as several days hosting costs.

@quooston
Copy link

quooston commented Nov 9, 2020

Yuck!

@steffen-wilke
Copy link

Use the property pattern to replace IsNullorEmpty

Please don't.

@jakubkeller
Copy link

I'm all for using the new features, but... why would I prefer this:

hello is { Length: >0 }

over a simpler pre-C#9 approach?

hello?.Length > 0 

You can then specify multiple criteria, not just Length.

@R2D221
Copy link

R2D221 commented Nov 11, 2020

You can then specify multiple criteria, not just Length.

Maybe, but the example doesn't show that. It proposes the new syntax as a replacement for string.IsNullOrEmpty.

Now that I think about it, what other criteria would you be able to get from string, anyway?

@kernelcoredump
Copy link

kernelcoredump commented Nov 11, 2020

You can then specify multiple criteria, not just Length.

You nailed it.
My first gut reaction to the example was also of harder readability and maintainability. Only when I thought about multiple property matches did it start to make sense.

@ebresafegaga
Copy link

ebresafegaga commented Nov 12, 2020

Language features become library functions in F#

let (|GT|_|) x y =
    if y > x then Some ()
    else None 

type String = { Length : int }

let test = function
    | { Length = GT 0  } -> "yes"
    | _ -> "No"

@MegaMax93
Copy link

Use the property pattern to replace IsNullorEmpty

Please don't.

+1

@TengshengHou
Copy link

if (greetings?[0] is {Length: > 0} hi)
我接受不了这样的语法。
I can't accept such a grammar.

@OJacot-Descombes
Copy link

The full power of these features becomes apparent when you combine them

if (obj is string { Length: > 0 } s) ...

This tests whether obj is a string (which implies that it is not null) and if it is, tests whether it is not empty, casts it to string and assigns it to a new variable s. And you can use this in a case label: switch(obj) ... case string { Length: > 0 } s:. This adds a lot of power to the switch statement and to the new switch expressions.

And you can apply these patterns recursively:

uiItem is RX.IIdCustom { IdQ: { Length: > 0 } } idCustom

@andrebmramos
Copy link

I'm all for using the new features, but... why would I prefer this:

hello is { Length: >0 }

over a simpler pre-C#9 approach?

hello?.Length > 0 

In the sample, he uses "is" in order to immediately create "hi" variable and use it in the next line.

@OJacot-Descombes
Copy link

@andrebmramos

With pre-C# 9 approach:

if (greetings[i]?.Length > 0) {
    string hi = greetings[i];
    // ...
}

With C# 9 pattern matching:

if (greetings[i] is { Length: > 0 } hi) {
    // ...
}

and you can use it in a switch statement or switch expression:

switch (greetings[i]) {
    case "hello":
        // ...
        break;
    case { Length: > 0 } hi:
        // ...
        break;
}

And you can integrate it in a recursive pattern as I've shown in my previous comment. So in simple cases the classic approach is just fine. In complex cases pattern matching can lead to terser code.

@banitt
Copy link

banitt commented Dec 14, 2020

IsNullOrEmpty way more readable for me.

@paulomorgado
Copy link

paulomorgado commented Dec 14, 2020

Surprisingly, pattern matching is more performant than string.IsNullOrEmpty.

Benchmark:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;

namespace StringTests
{
    class Program
    {
        static void Main(string[] args)
        {
            BenchmarkRunner.Run<Benchmark>();
        }
    }

    [MemoryDiagnoser]
    public class Benchmark
    {
        [Params(null, "", "text")]
        public string Text;

        [Benchmark]
        public bool StringIsNullOrEmpty() => string.IsNullOrEmpty(Text);

        [Benchmark]
        public bool PatternMatching() => Text is { Length: 0 };
    }
}
Method Text Mean Error StdDev Median Gen 0 Gen 1 Gen 2 Allocated
StringIsNullOrEmpty <null> 0.4166 ns 0.1147 ns 0.3217 ns 0.3279 ns - - - -
PatternMatching <null> 0.0023 ns 0.0050 ns 0.0142 ns 0.0000 ns - - - -
StringIsNullOrEmpty "" 0.3097 ns 0.0656 ns 0.1819 ns 0.2694 ns - - - -
PatternMatching "" 0.1479 ns 0.0605 ns 0.1655 ns 0.0942 ns - - - -
StringIsNullOrEmpty "text" 0.4161 ns 0.0515 ns 0.0738 ns 0.4036 ns - - - -
PatternMatching "text" 0.1712 ns 0.0755 ns 0.2155 ns 0.0795 ns - - - -

@jakubkeller
Copy link

Readability is key, through and through, to avoid unnecessary documentation also.
It would be a pain to have to wrap it in yet another function to compensate for lack of readability...

In the example, you would also have to test for whether or not Text was null correct? Or does is take care of the "null conditional"?

Is this necessary?
Text? is { Length: > 0 }

@OJacot-Descombes
Copy link

OJacot-Descombes commented Dec 15, 2020

@jakubkeller The property pattern automatically tests for the object to be not null. Therefore, you can also test for not null like this:
Text is { } t (which assigns the text to the new variable t at the same time.).

You could also test Text is string t; however, since Text is typed as string, a type test seems obsolete here. Or you can test Text is not null when you don't want to assign the result to a variable. The var pattern, on the other hand, matches even when the value is null: Text is var t.

@guillermosaez
Copy link

Use the property pattern to replace IsNullorEmpty

Please don't.

+1

+2

@mehdimiri
Copy link

if (userInput.KeyChar is 'Y' or 'y')
Thanks, I hate it .

@msenger1987
Copy link

Hi,
I am not sure if NRT is a new feature in C#8.0 or C#9.0.
Can someone make it clear.
thanks

@LukasKubicek
Copy link

Here is some well articulated critique for using pattern matching as replacement for IsNullOrEmpty:

https://endjin.com/blog/2020/12/dotnet-csharp-9-patterns-mechanism-over-intent

It's less readable and doesn't covney intent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment