Hacker News new | ask | show | jobs
by gouranga 5114 days ago
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.
1 comments

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!');
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.

> 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).