CSharpFunctionalExtensions
CSharpFunctionalExtensions copied to clipboard
Maybe.TryFirst() performance side-effect
This method is causing me some trouble when I'm using it inside a query (CQRS-wise) when source is an IQueryable<T>
.
public static Maybe<T> TryFirst<T>(this IEnumerable<T> source)
{
source = source as ICollection<T> ?? source.ToList();
if (source.Any()) return Maybe<T>.From(source.First());
return Maybe<T>.None;
}
Since IQueryable<T>
is not an ICollection<T>
the call to .ToList()
causes the whole database table to be fetched instead of only the first element (this is an issue when the table is really big). I understand the goal of calling ToList()
is to optimize the subsequent calls to Any()
and First()
but at the cost of a nasty performance side-effect.
As far as I understand, this method could even be removed (or marked deprecated) because it is doing nothing more than what is already possible by calling IEnumerable<T>.FirstOrDefault()
coupled with the implicit cast operator from T
to Maybe<T>
?
I agree with you, I implemented an Async version of this specifically for IQueryable
because I'm doing alot of EFCore work at the moment:
https://github.com/vkhorikov/CSharpFunctionalExtensions/issues/473
public static async Task<Maybe<T>> TryFirstAsync<T>(this IQueryable<T> source, CancellationToken cancellationToken = default)
{
var firstOrNull = await source.FirstOrDefaultAsync(cancellationToken);
if (firstOrNull == null)
{
return Maybe<T>.None;
}
return Maybe<T>.From(firstOrNull);
}
I did this ^^
What I did not like about the above code was the (3) calls , when it could be done in (1) call like @leubac said.
All you need is a .FirstOrDefault()
, then check for null, and if not null, return the value. If null, return the result.
@leubac - we can wait until one of the contributors comments, but honestly i'd just make a PR.
I opened an issue on mine because mine uses IQueryable
to take advantage of the IQueryable
async
methods
Good catch on this one, I saw the code a few hours ago, but for some reason it did not 'click' in my head to refactor it
Note that the fix here is to use another extension method that works on top of IQueriable
, like the one @jeffward01 suggested. Any method working on top of IEnumerable
(with any implementation) will prompt EF Core to load the full data set into memory.