Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IQueryable - TryFirst (async) -- Replacement for .FirstOrDefaultAsync() -- Is this helpful for a PR? #473

Open
jeffward01 opened this issue Dec 2, 2022 · 8 comments

Comments

@jeffward01
Copy link

jeffward01 commented Dec 2, 2022

Hello all,

I am new to this library, I am working with EFCore. In my scenerio I am working with IQueryable<T>.

I wanted a method like .FirstOrDefaultAsync(), because when I used .FirstOrDefaultAsync(), my return object was a pain, and looked like: Result<Maybe<TEntity?>, Error> , the nullable ? was giving me warnings in code.

I saw the methods TryFirst and TryLast - success I thought! But then I realized that these are not async, and do not use IQueryable

I created these methods for async and IQueryable:

    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);
    }

    public static async Task<Maybe<T>> TryFirstAsync<T>(
        this IQueryable<T> source,
        Func<T, bool> predicate,
        CancellationToken cancellationToken = default)
    {
        var firstOrEmpty = source.AsEnumerable()
            .Where(predicate)
            .Take(1)
            .AsQueryable();

        var firstOrNull = await firstOrEmpty.FirstOrDefaultAsync(cancellationToken);

        if (firstOrNull == null)
        {
            return Maybe<T>.None;
        }

        return Maybe<T>.From(firstOrNull);
    }

Note

Note: I realize that we do not use the Async suffix, however, using the suffix made things must easier, because without the suffix, the method would default to the IEnumerable<T> version, when I wanted to use the IQueryable<T> version.

Question

Is this helpful at all?

What should I do to replace IQueryable<T>.FirstOrDefaultAsync() in the future if this code is not helpful?

@jeffward01
Copy link
Author

jeffward01 commented Dec 2, 2022

Perhaps this would be better in a new nuget extension package called:

CSharpFunctionalExtensions.Extensions.IQueryable ?

The thing is:

  • Yes IEnumerable and IQueryable are practically the same
  • CSharpFunctionalExtensions works on IEnumerable
  • By using IQueryable, you get extra extension methods like AnyAsync() or .ToListAsync() (just some examples) that are not availble when using IEnumerable.
  • IEnumerable is used practically everywhere

I understand why you went the IEnumerable route, but I do think avoiding the benefits of IQueryable when your using a technology which uses IQueryable is something that can be improved on

@jeffward01
Copy link
Author

The comments on this SO post have some good information about the benefits of IQueryable in this insteance: https://stackoverflow.com/a/53417955/5022269

Comment Question:

Ok, I think my question is bad formed. I know that IQueryable deffer execution of the query until it is converted or used in foreach. My question is about if retrieving the source of the data is a long-running proccess (like database connection), if I use an IQueryable or IQueryable in foreach, the thread is blocked until I get the result? Can I start asyncronly the query execution before use it in the foreach? Are calling IQueryable.ToAsyncEnumerable() or IQueryable.ToListAsync() the right way to start the query execution before use it in a foreach? – Ivan Montilla

Comment Reply:

await xxx.ToListAsync() will case your query to be executed asynchronously, i.e. awaiting on this method will cause your thread to return to the thread pool and be available to run other stuff until the query executes and result is returned to the database. You code foreach (var customer in await customers) does exactly this. So yes, it is the right way to do it. – Nick
Nov 22, 2018 at 15:58

@maxime-poulain
Copy link

Lucian Wischik, who worked on the compiler for async/await pattern is clear:

I/O bound operations should always be asynchronous.

So everyting you say regarding having API with Task returning method when the input is an IQueryable is right.
https://youtu.be/OpOaiz4mNP0?t=466 (The guy himself explains it there if you are interessted).

BTW
FirstOrDefaultAsync has an overload that takes an Expression.
You can have

        public static async Task<Maybe<T>> TryFirstAsync<T>(
            this IQueryable<T> source,
            Expression<Func<T, bool>> predicate,
            CancellationToken cancellationToken = default)
        {
            var firstOrNull = await source.FirstOrDefaultAsync(predicate, cancellationToken);

            if (firstOrNull == null)
            {
                return Maybe<T>.None;
            }

            return Maybe<T>.From(firstOrNull);
        }
    // Usage : IQuery<string> query = XXX;
    // var item = await query.TryFirstAsync(x => x == "hello");

@vkhorikov
Copy link
Owner

vkhorikov commented Dec 13, 2022

We can add these 2 extension methods if we don't have them yet. Async postfix is fine to make it consistent with EF Core.

Regarding your second overload: please use @maxime-poulain 's version. This code:

var firstOrEmpty = source.AsEnumerable()
            .Where(predicate)
            .Take(1)
            .AsQueryable();

doesn't work because converting the source to IEnumerable with AsEnumerable() will prompt EF Core to load the whole set of items and then do the filtration in-memory, which defeats the purpose of using IQueriable.

@maxime-poulain
Copy link

Perhaps this would be better in a new nuget extension package called:
CSharpFunctionalExtensions.Extensions.IQueryable ?

I was looking at opening a PR with recent proposal/request in the issue part and
I realized the following.

While IQueryable is part of the framework(System.Linq.Expressions.dll), extension method FirstOrDefaultAsync belong to
the EF Core assembly.

I assume @vkhorikov (to him to confirm or not) doesn't want to bother to maintain a CSharpFunctionalExtensions.Extensions.IQueryable or CSharpFunctionalExtensions.Extensions.EfCore.

Depending on the answer, you can either close the ticket or create that NuGet :).

@vkhorikov
Copy link
Owner

Ah, indeed. I forgot it'd require a new dependency. We definitely don't want the library to depend on EF Core.

@jeffward01
Copy link
Author

Ah, indeed. I forgot it'd require a new dependency. We definitely don't want the library to depend on EF Core.

Perhaps we can group this into the set of 'new projects' PR that will split CSharpFunctionalExtensions into 'sub-packages', such as CSharpFunctionalExtensions.EfCore?

@vkhorikov
Copy link
Owner

Maybe. But that's something someone else would need to do, I don't have much time nowadays, unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants