Few days ago I stopped myself while writing code. I wrote my LINQ filtering wrong.
items.Where(x => x > 0).Any();
(Obviously, it’s a pseudo code)
I realized my mistake immediately, changed the code, and went on with my day.
Then it started bugging me. What other LINQ methods could I have misused in the similar way? Not noticing the mistake and thinking I picked the best solution for the problem I was facing?
I started digging around in documentation, in my and others’ code bases, and here are some common LINQ mistakes that I have found.
1. Single() vs First()
// Incorrect
var vehicle1 = vehicles.Single(x => x.Id == 42);
var vehicle2 = vehicles.SingleOrDefault(x => x.Id == 42);
// Correct
var vehicle1 = vehicles.First(x => x.Id == 42);
var vehicle2 = vehicles.FirstOrDefault(x => x.Id == 42);
Why? What’s the difference?
- Incorrect:
Single() or SingleOrDefault() makes sure that there is just one and only one match in the set of vehicles — there are no two or more vehicles with Id 42.
- Correct:
First() or FirstOrDefault() returns the first match — first vehicle with Id 42.
In most cases you are not ensuring that there is just one match. Often, that shouldn’t be the case — your primary key (Id) should be unique.
In terms of SQL it would look like this:
Single() or SingleOrDefault() — SELECT 2 …First() or FirstOrDefault() — SELECT 1 …
You can already see where this is going — the first one should have worse performance.
What about benchmarks (which you can run yourself here)?
Sample size of 10,000
Now we have some proof — use First() or FirstOrDefault(). Grab the Single() and SingleOrDefault() only when you really need it.
2. Where().First()
// Incorrect
var vehicle1 = vehicles
.Where(x => x.Brand == "Porsche")
.First();
var vehicle2 = vehicles
.Where(x => x.Brand == "Porsche")
.FirstOrDefault();
// Correct
var vehicle1 = vehicles.First(x => x.Brand == "Porsche");
var vehicle2 = vehicles.FirstOrDefault(x => x.Brand == "Porsche");
What’s the difference this time?
- Incorrect: First, get all the vehicles with the brand that equals to “Porsche”. Then retrieve the first of those vehicles.
- Correct: Get the first vehicle with the brand that equals to “Porsche” — then stop there.
Benchmark time? Benchmark time.
Sample size of 10,000
There is a small difference which proves the point — First() or FirstOrDefault() does the same as Where().First() — but with better performance.
3. Where().Where()
// Incorrect
var porscheVehicles = vehicles
.Where(x => x.Brand == "Porsche")
.Where(x => x.ProductionYear > 2010);
// Correct
var porscheVehicles = vehicles
.Where(x => x.Brand == "Porsche" && x.ProductionYear > 2010);
Explanation:
- Incorrect: Retrieve all the vehicles with brand Porsche, and out of them take vehicles with production year greater than 2010.
- Correct: Retrieve all vehicles with brand Porsche with production year greater than 2010.
In SQL it could end up like SELECT * FROM (SELECT FROM …).
And here we go — another benchmark.
Sample size 10,000
Contrary to most benchmarks where just numbers are used — this time it’s a benchmark that is similar to the example shown above — using vehicle objects. As a result, more memory is allocated. Code can be seen here.
As you can see, we have another proof — using just one Where is much more efficient.
4. OrderBy().OrderBy()
// Incorrect
var orderedVehicles = vehicles
.OrderBy(x => x.Brand)
.OrderBy(x => x.ProductionYear);
// Correct
var orderedVehicles = vehicles
.OrderBy(x => x.Brand)
.ThenBy(x => x.ProductionYear);
This is not a performance issue but instead a logical issue. What exactly is happening in the example above?
- Incorrect: Sort vehicles by brand. Then ignore the previous sort and re-sort the vehicles by production year.
- Correct: Sort vehicles by brand. Then take the previous sort into account and then also sort them by production year.
Note that the same applies to OrderByDescending() and ThenByDescending().
5. Empty Count()
// Incorrect
var count1 = vehiclesList.Count();
var count2 = vehiclesArray.Count();
// Correct
var count1 = vehiclesList.Count;
var count2 = vehiclesArray.Length;
What’s the problem here?
We shouldn’t be using Enumerable.Count() for arrays and any collections that implement ICollection<T> or ICollection interface — these include List<T> and Dictionary<T>.
We should be instead using Count property for ICollection implementation and Length for arrays.
Although, when we look into the LINQ source code we can see the following:
public static int Count<TSource>(this IEnumerable<TSource> source)
{
if (source == null)
throw Error.ArgumentNull("source");
ICollection<TSource> collectionoft = source as ICollection<TSource>;
if (collectionoft != null)
return collectionoft.Count;
ICollection collection = source as ICollection;
if (collection != null)
return collection.Count;
int count = 0;
using (IEnumerator<TSource> e = source.GetEnumerator())
{
checked
{
while (e.MoveNext()) count++;
}
}
return count;
}
When it can, it will early return Count instead of doing O(n) operation.
6. Count() instead of Any() or All()
// Incorrect
bool any1 = vehicles.Count() > 0;
bool any2 = vehicles.Count(x => x.Brand = "Porsche") > 0;
bool all = vehicles.Count(x => x.Brand = "Porsche") == vehicles.Count();
// Correct
bool any1 = vehicles.Any();
bool any2 = vehicles.Any(x => x.Brand = "Porsche");
bool all = vehicles.All(x => x.Brand = "Porsche");
Well, first of all I am curious if you noticed I used Count() instead of Count. Props to you if you did — that could be the first incorrect point.
(Although, from the small code snippet, it’s not obvious if it’s List<T> or not)
Difference?
- Incorrect: First do a full iteration of the (matching) vehicles. Then compare the amount.
- Correct: Check if there is any vehicle that matches the condition — then stop. In case of
All — stop when vehicle doesn’t match the condition.
When it comes to All there are other problems that we won’t get into now.
What about performance?
Sample size of 10,000
As you can see, operations with Count() are much slower.
Note that Count() compared to Any() is about the same — that’s because List<T> was used — Count() returns Count property.
When it comes to Any() and All() we managed to cut a lot of time. Note that, in the worst-case scenario these operations are O(n) as well and might take the same amount of time as using Count(). We should still rather use Any() and All() in hopes of making our code perform better.
Final words
While most LINQ providers optimize in most of the cases, it’s important to know about these limitations and differences. It’s important to know which LINQ method to use and when as it makes our code more readable and performs better.
I knew about most of these mistakes (That didn’t stop me from making them again and again). Did you? Do you know any other LINQ mistakes that people make often? Share them with us.
Remember, you can run all of the benchmarks yourself here.