akamsteeg / AtleX.HaveIBeenPwned

A fully async .NET Standard client library for the API of HaveIBeenPwned.com
https://www.nuget.org/packages/AtleX.HaveIBeenPwned/
MIT License
5 stars 0 forks source link

Internally, rely on nullable reference types and skip argument null checks #91

Open akamsteeg opened 9 months ago

akamsteeg commented 9 months ago

We have a lot of methods in internal or public sealed classes that are doing null checks on arguments, before passing the values to internal methods. Internally, we should rely on the correct usage of nullable reference types and argument checks in public (as in, part of the lib's public API) methods. If we skip the null checks, we can reduce code size and increase the chance of inlining. Aditionally we perform less work consuming less time, less CPU cycles and less energy.

akamsteeg commented 9 months ago

Personally, I'd like to treat any type boundary as hostile and verify all the input. But after the introduction of nullable reference types this situation changes a bit with regards to strictly internal types. In internal classes we're safe to omit the checks as long as the caller validates the arguments. We must be very, very careful here though. It is super easy to forget an argument check and end up with a NRE bug.

We could consider using Debug.Assert() on the internal stuff, but that doesn't play nice at the moment with nullable reference types. But still, it would save a bunch of argument checks everywhere.