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.
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.
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?
Name our custom methods appropriately so that it gives the idea for the caller that it does bode well or not with Linq
?
Move lazy methods to a different namespace, and keep Linq
-ish to another, so that it gives some sort of an idea at least?
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 IEnumerable
s, 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)..
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:
null
an acceptable input value?null
an acceptable element value?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.
Use IEnumerable wherever it makes sense, and code defensively.
As SLaks pointed out in a comment, deferred execution has been possible with IEnumerable
since the beginning, and since C# 2.0 introduced the yield
statement, it's been very easy to implement deferred execution yourself. For example, this method returns an IEnumerable that uses deferred execution to return some random numbers:
public static IEnumerable<int> RandomSequence(int length)
{
Random rng = new Random();
for (int i = 0; i < length; i++) {
Console.WriteLine("deferred execution!");
yield return rng.Next();
}
}
So whenever you use foreach
to loop over an IEnumerable, you have to assume that anything could happen in between iterations. It could even throw an exception, so you may want to put the foreach loop inside a try/finally
.
If the caller passes in an IEnumerable that does something dangerous or never stops returning numbers (an infinite sequence), it's not your fault. You don't have to detect it and throw an error; just add enough exception handlers so that your method can clean up after itself in the event something goes wrong. In the case of something simple like Shuffle
, there's nothing to do; just let the caller deal with the exception.
In the rare case that your method really can't deal with an infinite sequence, consider accepting a different type like IList
. But even IList won't protect you from deferred execution - you don't know what class is implementing IList or what sort of voodoo it's doing to come up with each element! In the super-rare case that you really can't allow any unexpected code to run while you iterate, you should be accepting an array, not any kind of interface.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With