rem
rem

Reputation: 17045

Checking for null in a collection

Which way of null checking would be better to use and why?

var myVar = myCollection.FirstOrDefault(q => q.Id == 10);
if (myVar != null)
{
   anotherVar = myVar.MyName;
}

or:

var myVar = myCollection.Where(q => q.Id == 10);
if (myVar.Any())
{
   anotherVar = myVar.First().MyName;
}

or just no difference?

Upvotes: 2

Views: 221

Answers (6)

Filippo Vitale
Filippo Vitale

Reputation: 8103

The two approches are different, here the ILs:

FirstOrDefault + if (myVar != null)

IL_0067:  ldsfld      UserQuery.CS$<>9__CachedAnonymousMethodDelegate1
IL_006C:  brtrue.s    IL_007F
IL_006E:  ldnull      
IL_006F:  ldftn       b__0
IL_0075:  newobj      System.Func<<>f__AnonymousType0<System.Int32,System.String>,System.Boolean>..ctor
IL_007A:  stsfld      UserQuery.CS$<>9__CachedAnonymousMethodDelegate1
IL_007F:  ldsfld      UserQuery.CS$<>9__CachedAnonymousMethodDelegate1
IL_0084:  call        System.Linq.Enumerable.FirstOrDefault  <-----
IL_0089:  stloc.2     // myVar
IL_008A:  ldloc.2     // myVar
IL_008B:  brfalse.s   IL_0094
IL_008D:  ldloc.2     // myVar
IL_008E:  callvirt    <>f__AnonymousType0<System.Int32,System.String>.get_MyName
IL_0093:  stloc.1     // anotherVar
IL_0094:  ldloc.1     // anotherVar
  • FirstOrDefault

Where + if (myVar.Any())

IL_0067:  ldsfld      UserQuery.CS$<>9__CachedAnonymousMethodDelegate1
IL_006C:  brtrue.s    IL_007F
IL_006E:  ldnull      
IL_006F:  ldftn       b__0
IL_0075:  newobj      System.Func<<>f__AnonymousType0<System.Int32,System.String>,System.Boolean>..ctor
IL_007A:  stsfld      UserQuery.CS$<>9__CachedAnonymousMethodDelegate1
IL_007F:  ldsfld      UserQuery.CS$<>9__CachedAnonymousMethodDelegate1
IL_0084:  call        System.Linq.Enumerable.Where   <-----
IL_0089:  stloc.2     // myVar
IL_008A:  ldloc.2     // myVar
IL_008B:  call        System.Linq.Enumerable.Any     <-----
IL_0090:  brfalse.s   IL_009E
IL_0092:  ldloc.2     // myVar
IL_0093:  call        System.Linq.Enumerable.First   <-----
IL_0098:  callvirt    <>f__AnonymousType0<System.Int32,System.String>.get_MyName
IL_009D:  stloc.1     // anotherVar
IL_009E:  ldloc.1     // anotherVar
  • Where
  • Any
  • First

It looks like micro-optimization but the first one should be faster because the single enumeration at FirstOrDefault but if the enumerator after the Where contains not many element with q.Id == 10 it doesn't really matter. Between the two I would definitely prefer the most clear syntax.

BTW I a mot a big fan of null... what about usign if (myVar != default(T)) instead?

Upvotes: 0

Servy
Servy

Reputation: 203802

The first option can be broken as a result of a null item that passes the check, thus making you think there are no matching items, even if they are. It doesn't apply to this particular example, but it could apply in the general case.

However, the second example here iterates the source sequence twice (sometimes), once to see if there are any results and then again to get that result. If the source sequence needs to perform a database query to get the results that could be very expensive. Because of this you should only use this option if you're sure that you have an in-memory collection you're dealing with and that it's not particularly large (or that the first item you need will be found quickly).

In the event that you need to worry about this particular edge case with the first option, or you want to get the benefits of using Any and First (for their superior semantic representation of what you want) with the performance benefits of FirstOrDefault you can use this pattern:

var myVar = myCollection.Where(q => q.Id == 10)
    .Take(1)
    .ToList();
if (myVar.Any())
{
   anotherVar = myVar.First().MyName;
}

You could make an extension method to shorten this if you wanted:

public static IEnumerable<T> FirstOrEmpty<T>(this IEnumerable<T> source)
{
    //TODO: null check arguments
    using (var iterator = source.GetEnumerator())
    {
        if (iterator.MoveNext())
            return new T[] { iterator.Current };
        else
            return Enumerable.Empty<T>();
    }
}

public static IEnumerable<T> FirstOrEmpty<T>(this IEnumerable<T> source, Func<T, bool> predicate)
{
    return FirstOrEmpty(source.Where(predicate));
}

This would allow you to just write:

var myVar = myCollection.FirstOrEmpty(q => q.Id == 10);
if (myVar.Any())
{
   anotherVar = myVar.First().MyName;
}

Upvotes: 1

Rapha&#235;l Althaus
Rapha&#235;l Althaus

Reputation: 60493

You want one element.

So one FirstOrDefault() with a null check is clearer than

Where
Any
First

For performance, this won't change your life in most cases. But I would take first for "readability".

Upvotes: 1

SilentDoc
SilentDoc

Reputation: 357

I tend to use expressions that involve Where followed by FirstOrDefault:

var myVar = myCollection.Where(x => x.Id==10).FirstOrDefault();
if (myVar != null)
{
   anotherVar = myVar.MyName;
}

As Raphael Althaus points out above, you want one variable to be null checked, in my opinion you should first make a query with the condition and then pick the first one if exists, and check that.

Upvotes: 0

Femaref
Femaref

Reputation: 61427

I'd prefer the first way, as it is much clearer what you intend to do.

Upvotes: 1

Tim Schmelter
Tim Schmelter

Reputation: 460018

Could be premature optimization, but the first approach needs only one execution hence it's a little bit more efficient.

I'm using the second approach when i might need the query later again lazily. The FirstOrDefault "finishes" it.

Upvotes: 1

Related Questions