Code Analysis in .NET: What is Missing
In my job, I do a lot of code reviews for the customers of my company.
Essential for this activity is the usage of Code Analysis. I have already written multiple posts about code analysis, the last one on .NET 6.
Still, based on my experience, Code Analysis are still missing some important checks, that I usually do in my code reviews.
-
Missing unhooked event handlers
Essentially, this means that every time there is a class hooking an event handler with +=, there must be a correspondent unhook via -=. Not doing so can raise memory leaks in applications (see for example Weak event patterns, but this is a global concept, not specific to WPF applications).
So it's fundamental to unhook event handlers. -
Non-compiled, instances RegEx
If the application code has always to use the same regular expression, it's better to create it as compiled and save it as a static variable in your code.
Otherwise, the regular expression must be compiled at each invokation, and not saving this instance in a class field, will require that next time all the work will be done again.
In the case that the regular expression is invoked many times, this can seriously affect the performance of the application.
Note: in .NET 7 there is the introduction of source code generated RegEx. These are even better than compiled RegEx! -
async void code must be protected with try/catch
Asynchronous methods should always return Task, Task<T>, ValueTask or ValueTask<T>. But this is not possible if you need to invoke async methods from an event handler. In this case it's ok to have the async method to return void.
But the calling code can't await for this method, and above all can't catch exceptions.
So, what happens if an exception is thrown in a async void method? Simply, the application crashes.
In reality, some type of applications can register a global exception handler. This could prevent the crash of the application. But still, code (above all in the case of a library) should not rely on it - i.e., it should be safe. -
Avoiding creating the async state machine, when possible
In case you have a method with a single await operation, and this operation is the last of your method, you don't need to await it.
You can instead simply return the Task returned from the contained method, without the await and so they async in the signature. Removing the async removed the need for the compiler to create the async state machine.
This reflects in faster compilation time and smaller generated code.