Reputation: 22576
I have the following code which a colleague of mine told me is incorrect and will crash if my variable is null:
List<FSKUser> users = null;
if (users == null || users.Count() == 0)
{
return false;
}
Obviously the =null
is just for testing purpose. But when I run this code it runs correctly and returns false.
Is the way I'm checking a safe and correct way to check?
Upvotes: 5
Views: 14596
Reputation: 464
may be this would be useful in your example:
List<FSKUser> users = null;
return users != null && users.Any();
Upvotes: 0
Reputation: 63772
Yes, this is the safe and correct way. Your colleague probably has some weird understanding of operator precedence or boolean expression evaluation in C# :)
The ||
operator (the same as &&
) will stop evaluating as soon as it can be sure of the results. Since boolean OR can never yield false
as soon as one of the operands is true
, it will either fail on the first operand (if it's null, the result is true
=> you're done), or it will evaluate both of the operators.
Of course, if you're not averse to using extension methods, this can be handily used to simplify the condition. Eg. you can use an extension method like this:
public static bool IsEmpty<T>(List<T> @this)
{
return @this == null || @this.Count == 0;
}
Which then lets you use a condition like this:
if (users.IsEmpty())
{
...
}
Also, note that List<T>
has a Count
property - you should probably use that instead of the extension method Count()
. In the end, it will do the same thing IIRC (it checks if the enumerable is a collection or a list, IIRC), but it goes through some loops to do that.
You might want to ask your colleague what he thinks will happen. You've got a simple test case that shows you're right, but perhaps he's got some reasons of his own why he doesn't want that. The most likely thing, however, is that he's used to a different programming language, and not actually a native C#-er. In that case, you've both gotten an opportunity to learn :)
Upvotes: 10
Reputation: 1064254
Your friend is wrong. The ||
operator short-circuits - it exits with true
the first time any term returns true
. Similarly, the &&
operator exits with false
the first time any term returns false
. As such, your users.Count()
cannot be reached if users
is null
(unless, perhaps, users
is a field and you are doing lots of threading, and the compiler and JIT both choose to explicitly load the field twice for some reason).
Upvotes: 3
Reputation: 22511
There is no problem with your code. The 2nd condition will only be evaluated if users does not equal null
, so this is the correct way to check against null.
In contrast to other languages (VB.NET and the And
and Or
operator as opposed to AndAlso
and OrElse
), C# only evaluates conditions as long as their result has an impact on the overall result. If - as in your case - the first condition of an Logical Or-operation already evaluates to true, there is no need to check the second one.
Upvotes: 0