Zapnologica
Zapnologica

Reputation: 22576

Correct way to check if a variable is null or empty

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

Answers (4)

BigMan
BigMan

Reputation: 464

may be this would be useful in your example:

List<FSKUser> users = null;
return users != null && users.Any();

Upvotes: 0

Luaan
Luaan

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

Marc Gravell
Marc Gravell

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

Markus
Markus

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

Related Questions