asynchronous - Best way to write an async method in C#

Question

I have a service object that simply executes queries against an EF Core repository on top of a Microsoft SQL Database. I am trying to convert these synchronous methods to async versions. Here's an example of one simplified method ...

Synchronous Version

public List<Contact> GetContacts()
{
    var ret = this.dbContext.Contacts.ToList(); 
    return ret;
}

I have seen several way to NOT create ASYNC methods ... then again ... I've seen a couple different ways on how you SHOULD create async methods. I am caught between which of the two methods below is considered the "right" or "best" way to create an async methods.

Async Version 1

public Task<List<Contact>> GetContactsAsync()
{
    return Task.FromResult(this.dbContext.Contacts.ToList());
}

OR

Async Version 2

public async Task<List<Contact>> GetContactsAsync()
{
    List<Contact> ret = null;

    await Task.Run(()=>{
        ret = this.dbContext.Contacts.ToList(); 
    });

    return ret;
}

Potentially complicating this further is, if the query contains several lines of logic.

Here is the same method with more of the detail included. If you see issues aside from the async topic, then I would love to hear about them in a PM or something, but for this thread I want to focus on the await/async stuff.

Complete Synchronous Code

public PaginationResult<Contact> GetContacts(PaginationSpec pagingSpec, List<Expression<Func<Models.Contact, bool>>> filter = null)
{
    IQueryable<Contact> query = ContactService.GetContactQuery(this.ctx);

    if (filter != null && filter.Count > 0)
    {
        foreach (var item in filter)
        {
            query = query.Where(item);
        }
    }

    // get the record count
    //
    var recordcount = query.Count();

    // get the pagination
    //
    var stage = pagingSpec.ApplyPagination<Models.Contact>(query);

    // Construct the paged result.
    // 
    var ret = new PaginationResult<Models.Contact>(recordcount, pagingSpec.CurrentPage, pagingSpec.PageSize, stage.ToList());

    return ret;
}

How would I wrap this in the appropriate syntax to make this method asynchronous? I can think of a couple of ways ...

Async Version 1

public Task<PaginationResult<Contact>> GetContactsAsync(PaginationSpec pagingSpec, List<Expression<Func<Models.Contact, bool>>> filter = null)
{
    return Task.FromResult(GetContacts(pagingSpec, filter)) // simply call the synchronous method ??;
}

OR

Async Version 2

public async Task<PaginationResult<Contact>> GetContactsAsync()
{
    PaginationResult<Contact> ret = null;

    await Task.Run(()=>{
    
        // Encapsulate the execution code within the RUN method ??
        //
        IQueryable<Contact> query = ContactService.GetContactQuery(this.ctx);

        if (filter != null && filter.Count > 0)
        {
            foreach (var item in filter)
            {
                query = query.Where(item);
            }
        }

        // get the record count
        //
        var recordcount = query.Count();

        // Apply the pagination spec to the query
        //
        var stage = pagingSpec.ApplyPagination<Models.Contact>(query);

        // Construct the esult.
        // 
        ret = new PaginationResult<Models.Contact>(recordcount, pagingSpec.CurrentPage, pagingSpec.PageSize, stage.ToList());
        });

    return ret;
}

Are either of these NOT appropriate, BETTER than one or the other or am I missing a 3rd option?

Trying to learn this await/async stuff and any help is appreciated! Thanks!

question from:https://stackoverflow.com/questions/65865584/best-way-to-write-an-async-method-in-c-sharp

Answer 1

Both of those are generally considered incorrect.

Async Version 1

The problem with this one is that it is not actually asynchronous. It's just doing the same synchronous work and wrapping it in a Task<T>.

Async Version 2

The problem with this one is that it is asynchronous from the perspective of the caller, but synchronous in its implementation. It's just using Task.Run to run synchronous code on a thread pool thread so it appears asynchronous. This is what I call "fake asynchronous", which is OK (not ideal) for desktop apps, and certainly not recommended for ASP.NET apps.

A more proper solution would look like this:

public async Task<List<Contact>> GetContactsAsync()
{
    return await this.dbContext.Contacts.ToListAsync();
}

This is assuming you have a ToListAsync method available. Note the similarity to the synchronous version:

public List<Contact> GetContacts()
{
    return this.dbContext.Contacts.ToList(); 
}

Generally speaking, you want to start at the lowest level, i.e., the code that actually executes the IQueryable. In this case, I'm assuming that's ToList, but if ToList is your own method, then it might be whatever that method calls. Once you find the lowest-level code, then change that to use await, and let the async grow from there.