11

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?

7
  • Just saying that TryGetValue is not a new method at all. And yes, I'm for using the "Try Do Thing, Out Param" pattern in all places where it makes sense and saves on null checks. Don't look at null as your "enemy", instead validate all data the moment it enters the application and not later, use nullable reference types, life will be better. :)
    – AyCe
    May 20 at 22:10
  • 4
    I think this problem is best solved with an Option/Maybe type. That is not completely the case in C# but I think it has the null-conditional operator which can be used as just a little worse alternative.
    – skywalker
    May 20 at 22:11
  • 1
    isnt null reference the #1 bug root cause in modern programming? it pretty much is "the enemy"
    – Ewan
    May 21 at 12:21
  • @Ewan The #1 bug root cause is the programmer making mistakes. Let the language and compiler help you get things right, by not allowing null in places where it doesn't make sense. A "null default object" makes no sense if the reference is never supposed to be null in the first place and may just hide bugs. They really should have included non-nullability from the beginning, but I guess it's easy to say that now.
    – AyCe
    May 21 at 15:37
  • 2
    But it converts it to something that has to be caught, or the program crashes. A null value can get passed around quite a bit before something tries to assume it's a non-null value, at which point it's not entirely clear where the null came from in the first place.
    – chepner
    May 21 at 23:32

4 Answers 4

17

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.

4
  • 3
  • 1
    I like most of this answer, but this statement "Out parameters require mutability, because you have to create a variable in the calling code that the function mutates" isn't correct. In C#, you can inline the out variable declaration into the call, which avoids mutability issues.
    – Doc Brown
    May 21 at 7:28
  • 3
    In my experience, out params improve chaining, as a variable is only available when it passes a test. Never letting anything be null is only useful if done right. "Default-initializing" objects with a "null object" just hides errors. Non-nullable types are the real solution, as they force you to think about stuff that could otherwise be problematic, like where to initialize a value (as early as possible!).
    – AyCe
    May 21 at 15:40
  • "There are a few reasons this has fallen out of favor. " Do you have any statistics to back this up? Mutability claim makes no sense in C# -- first of all you are talking about re-assigning not mutability and all variables in C# are, well, variables, you can assign them multiple times. May 22 at 4:23
11

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.

4
  • 1
    This is all correct (+1). I think it is worth to mention that even with no concurrency involved, the above code sequence has a noteable drawback: it requires two dictionary lookups instead of one, which might badly influence performance, at least in certain use cases.
    – Doc Brown
    May 21 at 7:22
  • makes for a confusing example though
    – Ewan
    May 21 at 12:21
  • hah, love how i gave basically the same answer before but with better formatting :softwareengineering.stackexchange.com/questions/387674/…
    – Ewan
    May 21 at 17:18
  • Thanks for pointing this out.
    – MaxouMask
    May 23 at 9:16
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.

3
  • Also relevant for choosing between TryParse and Parse: Is it expected that the operation might fail? Depending on your use case, failing is perfectly OK, or something that should never happen (and if it does, throw an exception, show an error, log, etc.). And also, you should worry about correct logic and proper code more than about performance. Make it work, make it nice, make it fast.
    – AyCe
    May 21 at 15:46
  • @AyCe - sure, but you're missing the point: it's not about when to chose Parse/TryParse, but rather about why try-operation pattern was introduced to the language's standard library in the first place (to offer a choice to the devs). If you're parsing an input string, of course it's expected to fail, at least some of the time. This is not about which API to choose, but which API to build into your own designs, or otherwise, which approach to take to mitigate the fact that you're working in a language where almost anything can be null (famous "billion dollar mistake" of Hoare). May 21 at 17:14
  • @AyCe - but I gut what you're saying - if in your use case failure is expected, then it's not an exceptional circumstance, and as a general rule, exceptions are not the right tool to handle that. It's just that that's not the main issue I'm trying to address in this answer. I'm trying to point out that the try-operation pattern is not really meant to be a cure for null checks. May 21 at 17:24
-1

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

2
  • That is one case where I think doing async Task<int?> TryGetAbcAsync() is OK. Main thing is to communicate whether you do try-get or success/exception.
    – AyCe
    May 22 at 21:28
  • Yes, in such case you need to refactor your existing TryABC(out int i) so that they no longer has out and ref parameter first, before you migrate to async. Regarding OP's question, if we use the pattern too often, we will have too many out and ref methods ,and this could significantly slow your progress down during the migration to async.
    – Nam Le
    May 23 at 1:44

Not the answer you're looking for? Browse other questions tagged or ask your own question.