Extending LINQ to accept nullable enumerables

Question

While working with Linq extensions it's normal to see code like this:

IEnumerable<int> enumerable = GetEnumerable();
int sum = 0;
if (enumerable != null)
{
    sum = enumerable.Sum();
}

In order to enhance the code quality, I wrote the following extension method that checks for nullable enumerables and breaks the linq execution.

public static IEnumerable<T> IgnoreIfEmpty<T>(this IEnumerable<T> enumerable)
{
    if (enumerable == null) yield break;
    foreach (var item in enumerable)
    {
        yield return item;
    }
}

So, I can refactor the code to be like this:

var sum = GetEnumerable().IgnoreIfEmpty().Sum();

---

My questions now:

1. What penalties are associated with my extension method at runtime?
2. Is it's a good practice to extend linq that way?

Update: My target framework is: 3.5

Answer 1

What penalties are associated with my extension method at runtime?

Your extension method is transformed into a state-machine, so there's the minimal overhead of that, but that shouldn't be noticeable.

Is it's a good practice to extend linq that way?

In your question you state:

While working with Linq extensions it's normal to see code like this (insert enumerable null check here)

And I beg to differ. The common practice says don't return null where an IEnumerable<T> is expected. Most cases should return an empty collection (or IEnumerable), leaving null to the exceptional, because null is not empty. This would make your method entirely redundant. Use Enumerable.Empty<T> where needed.

Answer 2

1. You're going to have a method call overhead, it will be negligible unless you are running it in a tight loop or a performance criticial scenario. It's but a shadow in comparison to something like a database call or writing to a file system. Note that the method is probably not going to be inlined, since it's an enumerator.
2. It's all about readability / maintainability. What do I expect to happen when I see GetEnumerable().IgnoreIfEmpty().Sum();? In this case, it makes sense.

Note that with C# 6 we can use the following syntax: GetEnumerable()?.Sum() which returns an int?. You could write GetEnumerable()?.Sum() ?? 0 or GetEnumerable()?.Sum().GetValueOrDefault() to get a non-null integer that will default to zero.

If you are truly concerned with performance, you could also slightly refactor your method so that it's not an enumerator. This may increase the chance of inlining, although I have no idea of the 'arcane' logic of the JIT compiler:

public static IEnumerable<T> IgnoreIfEmpty<T>(this IEnumerable<T> enumerable)
{
    if (enumerable == null) return Enumerable.Empty<T>();
    return enumerable;
}

More generally about extending Linq, I think it is perfectly fine as long as the code makes sense. MSDN even has an article about it. If you look at the standard Where, Select methods in Linq, and forget about the performance optimizations they have in there, the methods are all mostly one-liner methods.

Answer 3

You can skip the additional extension method and use null coalescing operator - this is what it's for, and a one-time check for nullability should be a lot more efficient than another state machine:

IEnumerable<int> enumerable = GetEnumerable();
int sum = 0;

sum = (enumerable ?? Enumerable.Empty<int>()).Sum();

Answer 4

Most of the times we write a lot of code just because we are enchanted by the beauty of our creation - not because we really need it - and then we call it abstraction, reusability, extensibility, etc..

Is this raw piece less readable or less extensible or less reuseable or slower :

var sum = GetEnumerable().Where(a => a != null).Sum(); 

The less code you write - the less code you test - keep it simple. BTW - it is good to write extension methods if you can justify it.