Should we abandon the "if not null" pattern?

Question

I've seen that with the Dictionary API now has methods with bool TryGetValue<TKey, TValue>(TKey key, out TValue value) and I like this kind of methods because they're signaling to the other devs that you must not check the value but the output of the Try method.

And now that I've seen this I've a tendency to this for my own methods. Like if I've a method that could be returning nothing, I like to go the way of bool TryGet[blahblah](criteria, out var result) instead of doing the var result = Get[blahblha](criteria) and then check if(result != null){...}

The main advantage that I see with this technic is the drastic reduction of NullPointerException eventualities: If you're using the method as the convention is implying you should do and the fact that the out keyword imposes that the object is initialized before returning, then, when you access the outed value you should be fine. As opposed to just checking for null where if you simply forget to check for null, then you'll end up in a world of pain...

Am I the only one thinking like that ? What are the cons that I don't see with this technique?

Answer 1

There are a few reasons this has fallen out of favor. Out parameters require mutability, because you have to create a variable in the calling code that the function mutates. Out parameters are better than reference parameters as they at least explicitly notify the caller that the object will change, but out still has a lot of the downsides associated with passing things by reference. Out parameters also break chaining, which can be acceptable in some cases, but if you make it a pattern in code it makes things harder to follow.

The try get pattern also make it easier to fall into bad practices in code by violating "tell don't ask." Each call is now asking something about it's internal state then deciding to do something based on that state, but all the logic is outside that thing. There are times when this can be acceptable, but it should be avoided as a generally practice. Excessive use of this pattern makes logic harder to follow as objects get used in more places, or bugs to occur when logic gets duplicated.

If the main goal is to avoid null based exceptions, then the better solution is to never let anything be null. This can be achieved with non nullable types and null object patterns. This is also what .Net/C#/Microsoft are starting to encourage with defaulting warnings about possible nulls in new visual studio releases and supporting explicit marking of reference types that may be null.

Answer 2

The bool TryXXX() pattern is not designed to avoid nulls. A method could assign null to an out parameter so technically you would still have to check.

It's needed in locking situations, say where you have a ConcurrentDictionary you can't do:

if(dic.ContainsKey(key))
{
  return dic[key]
}

Because your check might return true, but then another thread removes the item. You cant just do dic[key] and check for null due to the ambiguity of having the key and a value of null vs not having the key.

c#8 should be forcing you to declare possible null returns as nullable types and then giving you lots of green underlines if you don't check for null. switch on warnings as errors and you should be good returning null.

Answer 3

This has been around since the early days of .Net - e.g., TryParse(string). In that particular case, it was, among other things, a way for the dev to indicate that an exception shouldn't be thrown. Internally, Parse interrupts normal execution flow by throwing an exception and incurs a performance hit, TryParse does not. This doesn't really matter if there's only an occasional Parse call, but if you're calling such a method a lot of times in a tight loop, you'll feel it.

For null checks, try-get-value is more of a band-aid, though - as it is a null check in (an unconvincing) disguise. It serves its band-aid purpose better if the API is in your control and you make it the only way to access the value, but this might be cumbersome, especially if the system is not designed in a "tell don't ask" manner.

It's better if you can make the return type non-nullable, or if you can leverage the null-object pattern (the simplest example of that is returning an empty list instead of null), or if you can leverage the Maybe/Option monad, or even if you focus your null checks on entry points, and write internal code as self-contained classes/functions that assume that the value they get is not null, even though it technically could be (except in cases where it's vital to check). The last one is helped by the presence of tests - which serve to increase your confidence that exceptions are thrown (or that there's some other potentially more appropriate form of handling) at all points where an invalid value can enter your system, and that the system beyond that is well-behaved.

P.S.
What you should be after regarding null handling is not merely to force your future self and other developers to perform null checks to avoid errors. This is a low bar. What you want is to take a more thought out approach to your error handling - you don't want your code to just be littered with checks and log calls. You want to be more deliberate about it. This lets you decluter your domain logic and write code that communicates better what is it that it's actually trying to do. It's not about hiding errors or pushing them under the rug, it's about being able to write more clearly - and if you can do that, then you're in a better possition to check if your overall logic is actually correct.

Answer 4

Another downside is that if later on you want to adopt asynchronous programming into your code base, you can't convert your TryABC(out int i) into TryABCAsync(out int i) because async method can't have out and ref parameters