Don't use methods with side-effects in LINQ
The author of this GitHub issue is complaining about the behavior or FirstOrDefault
when it's following an OrderBy
. Indeed, the predicate is called on each element which is not what he expected. In his example the predicate has a side-effect so he doesn't want the method to be called for each element.
var account = accounts.OrderBy(x => x.UsageCount).FirstOrDefault(x => x.TryReserve(token));
// TryReserve is called for each items even if the first one returns true
LINQ provides a declarative way to work with collections. By declarative, I mean that you express what you want but you don't know how it will be executed. Sometimes it may optimize multiple methods by combining them or change the execution order. It's very similar to SQL in that sense. Let's see an example that shows that the execution may be different from what you expect.
var result = Enumerable.Range(0, 10)
.FirstOrDefault(x =>
{
Console.WriteLine(x);
return x == 3;
});
Console.WriteLine("Result: " + result);
result
equals 3 as expected. The collection is enumerated until FirstOrDefault
finds an item that matches the condition.
Let's add OrderBy
before FirstOrDefault
and let's see what happens:
var result = Enumerable.Range(0, 10)
.OrderBy(x => x)
.FirstOrDefault(x =>
{
Console.WriteLine(x);
return x == 3;
});
Console.WriteLine("Result: " + result);
The result is still the expected one. However, the way the result is computed may not be the one you expect. Indeed, FirstOrDefault
process all items.
Let's split FirstOrDefault
to Where
and FirstOrDefault
:
var result = Enumerable.Range(0, 10)
.OrderBy(x => x)
.Where(x =>
{
Console.WriteLine(x);
return x == 3;
})
.FirstOrDefault();
Console.WriteLine("Result: " + result);
This time, the result is ok and it's executed as expected.
#Workaround
If you want to evaluate the predicate lazily (without any optimizations), you can create your extension method:
public static class EnumerableExtensions
{
public static T LazyFirstOrDefault<T>(IEnumerable<T> items, Predicate<T> predicate)
{
foreach (var item in items)
{
if (predicate(item))
return item;
}
return default;
}
}
Another way is to use the AsActualEnumerable()
extension method from Meziantou.Framework to prevent the optimization:
// Requires <PackageReference Include="Meziantou.Framework" Version="2.17.0" />
var result = Enumerable.Range(0, 10)
.OrderBy(x => x)
.AsActualEnumerable() // Wrap the enumerable into a custom IEnumerable<T> instance, so no *optimization* applies
.FirstOrDefault(x =>
{
Console.WriteLine(x);
return x == 3;
});
You can also convert your code to use foreach
loop. Starting with VS 15.7 Roslyn provides a code fix to convert from LINQ to foreach
and vice-versa as stated in this GitHub issue.
#Conclusion
When using LINQ it's important to understand that it's a descriptive language, not an imperative language. This means you will get the expected result but you don't know how. So, you don't know how the lambda you provide will be executed nor how many times. This means you should only use methods without side-effects to prevent weird behaviors.
Note that the documentation doesn't say anything about the actual implementation, nor about how the predicates will be used.
Last but not least, this issue is only one of the potential risks of LINQ. Other optimizations may change the way your code works, such as .Skip allows a Collection to be modified during enumeration.
#Additional resources
Do you have a question or a suggestion about this post? Contact me!