views:

391

answers:

3

I am a bit curious as to which is considered "best practice" when it comes to FirstOrDefault.

I've already seen this question, which is similar to the question I have, but not close enough that it answers my question.

Which of these is "better code"? and why?

var foos = GetMyEnumerableFoos();

var foo1 = (from f in foos
     where f.Bar == "spider monkey"
     select f).FirstOrDefault();

/* OR */

var foo2 = foos.FirstOrDefault(f => f.Bar == "spider monkey");

I tend toward the latter, as IMO, it makes for cleaner code. But I'm curious as to whether or not the techincal "guts" of what's going on there is more efficient the other way. Does this change if you're using different types of IEnumerables? like DataTables or string arrays or LINQ objects?

=========edit==========

Assuming Jon Skeet's post is correct, I went looking into Reflector to see what Where and FirstOrDefault look like, and here's what I came up with:

In the case of foos.Where(f => f.Bar == "spider monkey").FirstOrDefault()

public static IEnumerable<TSource> Where<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    if (source == null)
    {
        throw Error.ArgumentNull("source");
    }
    if (predicate == null)
    {
        throw Error.ArgumentNull("predicate");
    }
    if (source is Iterator<TSource>)
    {
        return ((Iterator<TSource>) source).Where(predicate);
    }
    if (source is TSource[])
    {
        return new WhereArrayIterator<TSource>((TSource[]) source, predicate);
    }
    if (source is List<TSource>)
    {
        return new WhereListIterator<TSource>((List<TSource>) source, predicate);
    }
    return new WhereEnumerableIterator<TSource>(source, predicate);
}

which would feed into:

public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source)
{
    if (source == null)
    {
        throw Error.ArgumentNull("source");
    }
    IList<TSource> list = source as IList<TSource>;
    if (list != null)
    {
        if (list.Count > 0)
        {
            return list[0];
        }
    }
    else
    {
        using (IEnumerator<TSource> enumerator = source.GetEnumerator())
        {
            if (enumerator.MoveNext())
            {
                return enumerator.Current;
            }
        }
    }
    return default(TSource);
}

In the case of foos.FirstOrDefault(f => f.Bar == "spider monkey");

public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    if (source == null)
    {
        throw Error.ArgumentNull("source");
    }
    if (predicate == null)
    {
        throw Error.ArgumentNull("predicate");
    }
    foreach (TSource local in source)
    {
        if (predicate(local))
        {
            return local;
        }
    }
    return default(TSource);
}

Looking at that still leaves me a bit confused, would there be efficiencies added by using the proper iterator for certain object types? Or is it more efficient to skip all that and just start looping through and testing? My gut again tells me it's the latter.

+7  A: 

Well, the "select" part will be removed by the compiler anyway, so you're really comparing:

foo.Where(f => f.Bar == "spider monkey")
   .FirstOrDefault()

vs

foo.FirstOrDefault(f => f.Bar == "spider monkey")

I doubt that it will make any significant difference to efficiency within LINQ to Objects anyway. I'd personally use the latter version myself... unless I wanted to reuse the filtering part of the query elsewhere.

Jon Skeet
That's interesting. I did a little more digging after you're post just to see what's going on in those three methods you mentioned. I posted my findings above... It certainly helps answer my question.
blesh
+3  A: 

I prefer the latter simply because it is more concise.

It terms of efficiency I doubt you could find a substantial difference between the 2. They are virtually identical in behavior even though in practice they work slightly different.

The biggest difference is that the first one creates a new IEnumerable<T> instance which is then walked for the first element. In the latter case, the existing IEnumerable<T> instance is walked for the first value matching the predicate. Again, very unlikely to be noticed.

JaredPar
A: 

The second version would be much more efficient.

The first version

var foo1 = (from f in foos
     where f.Bar == "spider monkey"
     select f).FirstOrDefault();

Will always loop through all items, creating a new collection of matching items.

The second version

var foo2 = foos.FirstOrDefault(f => f.Bar == "spider monkey");

will only loop through items until a match is found and then returned.

phdesign
Actually, if you put break point in your LINQ you'll see this... but it will only execute that LINQ statement once. you can even have LINQ statements that accessing IEnumerables created by LINQ statements ad nauseum and it'll still only go through them each once if you call FirstOrDefault.No worries, though, I used to think the same thing. ;)
blesh