Explicit null checking is a code smell in most cases. It should be
limited where possible, or pushed to the edge of the system. A common
anti pattern I've noticed is the incorrect use of
First() in LINQ,
which I have used myself on many occasions in this manner.
Assuming a collection of items that you wish to query, the incorrect approach is to explicitly check for a null return value and act accordingly.
The use of
FirstOrDefault() is redundant because no default is
actually set. The default value of a reference type would be null.
Meaning the explicit null checked is required. We could use
alone, but this will throw an exception if there are no elements to
A better solution is to set the default. As long as our initial query is not operating on a null reference this is safe. Here the explicit null check is gone. We have replaced it with a more functional solution which is after all what LINQ is based upon. While both are equivalent, the second example is much cleaner as well as being open to further chained statements.
First() relies on one or more items being in the sequence. When you
are only ever dealing with one result
Single() is more appropriate.
This method will throw an exception if more than one result is found,
acting as a form of assertion. Like
SingleOrDefault() which would work in the same manner as above.