Search code examples
c#linqparametersienumerabledesign-principles

What practices can safeguard against unexpected deferred execution with IEnumerable<T> as argument?


There are a few questions similar to this which deals with right input and output types like this. My question is what good practices, method naming, choosing parameter type, or similar can safeguard from deferred execution accidents?

These are most prevalent with IEnumerable which is a very common argument type because:

  • Follows the robustness principle "Be conservative in what you do, be liberal in what you accept from others"

  • Used extensively with Linq

  • IEnumerable is high in the collection hierarchy and predates newer collection types

However, it also introduces deferred execution. Now we might have gone wrong in designing our methods (especially extension methods) when we thought the best idea is to take the most basic type. So our methods looked like:

public static IEnumerable<T> Shuffle<T>(this IEnumerable<T> lstObject)
{
    foreach (T t in lstObject)
       //some fisher-yates may be
}

The danger obviously is when we mix the above function with lazy Linq and its so susceptible.

var query = foos.Select(p => p).Where(p => p).OrderBy(p => p); //doesn't execute
//but
var query = foos.Select(p => p).Where(p => p).Shuffle().OrderBy(p => p);
//the second line executes up to a point.

A bigger edit:

Reopening this: a criticism of a language's functionality isn't constructive - however asking for good practices is where StackOverflow shines. Updated the question to reflect this.

A big edit here :

To clarify the above line - My question is not about the second expression not getting evaluated, seriously not. Programmers know it. My worry is about Shuffle method actually executing the query up to that point. See the first query, where nothing gets executed. Now similarly when constructing another Linq expression (which should be executed later), our custom function is playing the spoilsport. In other words, how to let the caller know Shuffle is not the kinda function they would want at that point of Linq expression. I hope the point is driven home. Apologies! :) Though its as simple as going and inspecting the method, I'm asking how do you guys typically program defensively..

The above example may not be that dangerous, but you get the point. That is certain (custom) functions don't go well with the Linq idea of deferred execution. The problem is not just about performance, but also about unexpected side-effects.

But a function like this works magic with Linq:

public static IEnumerable<S> DistinctBy<S, T>(this IEnumerable<S> source, 
                                              Func<S, T> keySelector)
{
    HashSet<T> seenKeys = new HashSet<T>(); //credits Jon Skeet
    foreach (var element in source)
        if (seenKeys.Add(keySelector(element)))
            yield return element;
}

As you can see both the functions take IEnumerable<>, but the caller wouldn't know how the functions react. So what are the general cautionary measures that you guys take here?

  1. Name our custom methods appropriately so that it gives the idea for the caller that it does bode well or not with Linq?

  2. Move lazy methods to a different namespace, and keep Linq-ish to another, so that it gives some sort of an idea at least?

  3. Do not accept an IEnumerable as parameter for immediately executing methods but instead take a more derived type or a concrete type itself which thus leaves IEnumerable for lazy methods alone? This puts the burden on the caller to do the execution of possible un-executed expressions? This is quite possible for us, since outside Linq world we hardly deal with IEnumerables, and most basic collection classes implement up to ICollection at least.

Or anything else? I particularly like the 3rd option, and that's what I was going with, but thought to get your ideas prior to. I have seen plenty of code (nice little Linq like extension methods!) from even good programmers that accept IEnumerable and do a ToList() or something similar on them inside the method. I don't know how they cope with the side-effects..

Edit: After a downvote and an answer, I would like to clarify that its not about programmers not knowing about how Linq works (our proficiency could be at some level, but thats a different thing), but its that many functions were written not taking Linq into account back then. Now chaining an immediately executing method along with Linq extension methods make it dangerous. So my question is there a general guideline programmers follow to let the caller know what to use from Linq side and what not to? It's more about programming defensively than if-you-don't-know-to-use-it-then-we-can't-help! (or at least I believe)..


Solution

  • As you can see both the functions take IEnumerable<>, but the caller wouldn't know how the functions react.

    That's simply a matter of documentation. Look at the documentation for DistinctBy in MoreLINQ, which includes:

    This operator uses deferred execution and streams the results, although a set of already-seen keys is retained. If a key is seen multiple times, only the first element with that key is returned.

    Yes, it's important to know what a member does before you use it, and for things accepting/returning any kind of collection, there are various important things to know:

    • Will the collection be read immediately, or deferred?
    • Will the collection be streamed while results are returned?
    • If the declared collection type accepted is mutable, will the method try to mutate it?
    • If the declared collection type returned is mutable, will it actually be a mutable implementation?
    • Will the collection returned be changed by other actions (e.g. is it a read-only view on a collection which may be modified within the class)
    • Is null an acceptable input value?
    • Is null an acceptable element value?
    • Will the method ever return null?

    All of these things are worth considering - and most of them were worth considering long before LINQ.

    The moral is really, "Make sure you know how something behaves before you call it." That was true before LINQ, and LINQ hasn't changed it. It's just introduced two possibilities (deferred execution and streaming results) which were rarely present before.