[thomasz]

I don't see any pitfalls that wouldn't arise with imperative techniques. Well, Single() throws if there is no or more than one item with the property name "Blah!" in the collection, but if you just want the first one or none you should use FirstOrDefault() anyway. The point is that you need to write roughly a dozen lines with more room for hand grenades to reproduce that functionality with imperative constructs:

    Thing blah;
    bool found = false;
    foreach (var item in collection) {
        if (item.Property.Name == "Blah!") {
            if (!found) {
                found = true;
                blah = item;
            } else {
                throw new InvalidOperationException("collection contains more than one \"Blah!\" item!");
            }
        }
    }
    if (!found)
        throw new InvalidOperationException("collection does not contain a \"Blah\" item");

Come on, that's ridiculously verbose and doesn't solve any problems of the lambda version. If you are concerned about nulls, then you have to insert != null expressions anyway. And you can easily refactor by introducing an additional right above the old one,

    collection
        .Where(s => s != null && s.Property != null && s.Property.Name != null)
        .Where(s => s.Property.Name == "Blah!").Single()

while inserting the null checks into the imperative code takes considerably more effort (locating the predicate, making a decision weather or not to turn the predicate into a 120 char wide monster or adding another if block etc.)

[gouranga]

You miss one important point, which I've learned from many years of using .Net:

Which dereference in your LINQ expression is causing the NullReferenceException?

Try solving that problem when it goes pop in production and you have a couple of million quid in flight!

I'd write it as follows (probably wrapped as a generic function of T:

    public Thing GetItemWithName(ICollection<Thing> collection)
    {
        Thing output;
        int found;

        // preconditions
        Check.IsNotNull(collection, "collection was null");

        foreach(var item in collection)
        {
            // ref checks
            Check.IsNotNull(item, "item was null");
            Check.IsNotNull(item.Property, "item.Property was null");
            Check.IsNotNullOrWhiteSpace(item.Property.Name, "item.Property.Name was null or whitespace");
      
            if (string.Compare(item.Property.Name, "Blah!", StringComparison.InvariantCultureIgnoreCase) != 0)
                continue;
            
            found++;

            // rule check
            Check.IsTrue(found <= 1, "Found more than expected single element in collection");

            output = item;
        }

        // postconditions
        Check.IsNotNull(output, "output null. Expected reference");

        return thing;      
  }

Note: I tend to write proper industrial grade stuff that has to work every time without fail or any edge cases or conditions. These conditions are prescribed up front. Zero bugs and fail early is the only acceptable outcome which is why this is verbose.

[thomasz]

Well, if the error reporting of Enumerable.Single() isn't good enough for you (i think it throws InvalidOperationExceptions with different messages for both error cases), nothing stops you from implementing it once for yourself and profit from the benefits everytime instead of writing the same looping constructs interlaced with error reporting again and again.

    public static T MySingle<T>(
                this IEnumerable<T> collection, 
                Func<T,bool> pred,
                string foundNoneMsg,
                string notUniqueMsg) {
        Check.IsNotNull(collection, "collection was null");
        var filtered = items.Where(item => pred(item));
        Check.IsTrue(filtered.Any(), foundNoneMsg);
        Check.IsFalse(filtered.Skip(1).Any(), notUniqueMsg);
        return filtered.Single();
    }

    bool HasName(this Thing item, string name) {
         Check.IsNotNull(item, "item was null");
         Check.IsNotNull(item.Property, "item.Property was null");
         Check.IsNotNullOrWhiteSpace(item.Property.Name, "item.Property.Name was null");
         Check.IsNotNullOrWhiteSpace(name, "name was null or whitespace");
         return item.Property.Name
                .CompareTo(name, StringComparison.CultureInvariantIgnoreCase);
    }

    collection.MySingle(
            item => item.HasName("Blah!"),
            foundNoneMsg: "collection has no item with Property.Name 'Blah!'
            notUniqueMsg: "collection contained more than one 'Blah!');

[gouranga]

Your code iterated more than once. Mine doesn't. Mine is O(N). This is one of the side effects of using all the functionality.

On a positive note, your example is very lisp-like.

[thomasz]

> Your code iterated more than once.

Uh... no.

*edit The first element is iterated three times, the second two times and the rest once, making it O(n).