This is something that came up in the comments last week. I was refactoring some code and wound up with the accessor method below that performs multiple null checks while trying to assign a value to _currentUser.
// CurrentUser
private WebUser _currentUser;
public WebUser CurrentUser
{
get
{
if (_currentUser == null) _currentUser = GetWebUserFromSession();
if (_currentUser == null) _currentUser = GetWebUserFromTrackingCookie();
if (_currentUser == null) _currentUser = CreateNewWebUser();
return _currentUser;
}
}
Now this code is pretty clear, but a reader named Brian pointed out in a comment that we could shorten the code a bit by using the Null Coalescing operator “??”. If you haven’t used the Null Coalescing operator yet, check it out. It’s great for data access code where a method may return data or a null value. The basic syntax is illustrated in this block of code from msdn:
// y = x, unless x is null, in which case y = -1. int y = x ?? -1; // Assign i to return value of method, unless // return value is null, in which case assign // default value of int to i. int i = GetNullableInt() ?? default(int);
So, this is really cool, but there’s more. What Brian pointed out was that you can chain the ?? operator to do multiple null checks. So, my block of code above can be rewritten like this:
// CurrentUser
private WebUser _currentUser;
public WebUser CurrentUser
{
get
{
_currentUser = _currentUser ??
GetWebUserFromSession() ??
GetWebUserFromTrackingCookie() ??
CreateNewWebUser();
return _currentUser;
}
}
Note that normally the _currentUser assignment would fit all on one line but due to the width limitation of my blog I broke it up into multiple lines. So, C# Null Coalescing operator chaining. I like it. I’ll be adding it to my toolbox.
Addendum:
Hey, you guys pretty much hated this change to the code. I got no blog comments but I received quite a few emails about this technique and the prevailing opinion seems to be that the original syntax with the multiple if blocks was clearer. In fact, not a single person who emailed me liked the ?? method. So I took another look and I think I agree. The original code does seem a little clearer to me and I don’t really think I was able to make my code any shorter or simpler by using ??. So this may not have been the best example, but I do still think this is a very cool technique for the right situation. As always, I just need to make sure that I’m refactoring because the changes make the code cleaner, not just because they’re clever.
I think you should encapsulate the WebUser "find" strategy as well using a method that gets a lambda array passed an returns the first found WebUser.
ReplyDeleteSomething like this:
private WebUser FindWebUser(Func<WebUser>[] webUserFinders)
{
foreach (var webUserFinder in webUserFinders)
{
var webUser = webUserFinder();
if (webUser!=null)
{
return webUser;
}
}
throw new InvalidOperationException("Could not find a valid web user");
}
I was unaware of this operator, and I like it. While I agree with your addendum decision that the original code is more readable to the majority of developers, the null coalescing operator is clear once one internalizes its meaning, much like the ternary operator.
ReplyDeletePut me down in the 'like' category.
I like it better, though a compromise on readability between the two might be better:
ReplyDeleteif (null == _currentUser) { _currentUser = foo ?? bar ?? ...; }
return _currentUser;
Some people take a while to grok the coalesce operator (in any language). You can do the same thing in JavaScript with the logical or operator: var foo = var1 || var2 || bar || 0; Or in SQL with the coalesce and isnull functions: select coalesce(col1, col2, 42) from ... or select isnull(col1, 42) from ...
What I don't like though is 1) the lack of "this" usage and 2) the lack of defense programming.
Using "this" when referencing instance fields, properties, and methods keeps the code much more tidy, easier to understand by newcomers, lessens the chance of variable/scope confusion, and saves you a mouseover in your IDE to see where the variable originates.
The defensive programming technique of placing the constants or unassignables on the left side of an [in]equality statement keeps you from accidentially creating tautalogies: if (null = foo) vs if (null == foo)
One other syntax optimization I've found is to always start the line of a multiline statement with the operator instead of ending the line with it:
foo = bar
|| stuff
|| 42;
doStuff(
foo
, bar
, stuff
);
I've found this makes debugging and maintenance take less keystrokes, introduce less mistakes, and is easier to read due to the quick notice that it is a multiline statement, and exactly what the operators are.
But maybe that's just me.
PS - Is there some reason why can I not cut, copy, paste, or use arrow keys in this comment textarea?
I use the pattern quite often. As Xeno pointed out, the readavility can be improved (of course this is subjective) if the operator is at the BEGINNING of the line (where the eye tends to scan).
ReplyDeleteI like the coalescing operator better. Much less verbose without sacrificing readability. The only thing I can think of improving it is to return the result immediately without setting it to the _currentUser.
ReplyDelete// CurrentUser
private WebUser _currentUser;
public WebUser CurrentUser
{
get
{
return _currentUser ??
GetWebUserFromSession() ??
GetWebUserFromTrackingCookie() ??
CreateNewWebUser();
}
}
Nice post very helpful
ReplyDeleteDBAKings
perfect !!
ReplyDelete