Is this the correct way to write asynchronous methods?

Question

I'm currently trying to write async code and I have the feeling that my code is not too correct at all.

I have the following method:

public void Commit()
{
    _context.SaveChangesToDatabase();
}

Don't judge the code here as this are only samples. Also, don't say that if I'm using Entity Framework, that they come packaged with Async methods already. I just want to understand the async concept here.

Let's say that the method SaveChangesToDatabase does takes seconds to complete. Now, I don't want to wait for it so I create an async method:

public async Task CommitAsync()
{
    await Task.Run(() => Commit());
}

Does this mean that if I have a method:

public void Method()
{
    // Operation One:

    CommitAsync();

    // Operation Two.
}

Does this mean that my code on Operation two will be executed before CommitAsync() is even completed?

If not, please guide me in the right direction.

Update

Based on the remarks here that I'm ignoring my async method results, is this implementation better?

public Task<TaskResult> CommitAsync()
{
    var task = new Task<TaskResult>(() =>
    {
        try { Commit(); }
        catch (Exception ex)
        {
            return new TaskResult
            {
                Result = TaskExceutionResult.Failed,
                Message = ex.Message
            };
        }

        return new TaskResult { Result = TaskExceutionResult.Succeeded };
    });

    task.Start();
    return task;
}

This does mean that I need to put the async modifier on the method that call this code so that I can await this which means continue with the current execution and return when this method has been completed.

Answer 1

Fire but don't forget

CommitAsync() returns a Task, but Method ignores the return value of CommitAsync completely -- so yes, the code will not wait but simply go on with what's after that. This is bad, because, if Commit() throws an exception, you will never see it. Ideally, every task should be waited on somewhere by someone, so you can at least see if it fails.

Let's say that you have no async alternative to SaveChangesToDatabase, but you'd like to use it in an async context anyway. You can use Task.Run to create a "fake-asynchronous" method, but this is not recommended (see below):

public Task CommitAsync() {
    return Task.Run(() => Commit());
}

And then, assuming Method is doing something interesting with async (which the below code does not do since it's the only asynchronous operation in there):

public async Task MethodAsync() {
    // Operation One:

    await CommitAsync();

    // Operation Two.
}

Assuming you do not want to wait, but you do want to do something if the task failed, you can use a separate method:

public void Method() {
    // Operation One:

    var _ = TryCommitAsync();

    // Operation Two.
}

private async Task TryCommitAsync()
{
    try
    {
        await CommitAsync();
    }
    catch (Exception ex)
    {
        Console.WriteLine(
            "Committing failed in the background: {0}", 
            ex.Message
        );
    }
}

Getting back results

Let's suppose .Commit() does return something (like the number of records affected); a similar "fake-asynchronous" wrapper (again, not recommended - see below) would look like this:

public Task<int> CommitAsync() {
    return Task.Run(() => Commit());
}

If you want this result, you can await the task immediately:

public async Task MethodAsync() {
    // Operation One:

    int recordsAffected = await CommitAsync();

    // Operation Two.
}

Or, if you don't need it immediately, use await when you do:

public async Task MethodAsync() {
    // Operation One:

    Task<int> commit = CommitAsync();

    // Operation Two.

    // At this point I'd really like to know how many records were committed.
    int recordsAffected = await commit;
}

Async: don't fake it

In general, you don't want to write wrappers like CommitAsync() because they mislead callers into thinking code will be asynchronous when it isn't really, which brings few benefits other than not blocking (which is still useful in UI code, but not as good as true asynchronous code which doesn't need to use worker threads for everything). In other words, you should use Task.Run in the invocation of a method, not as the implementation of a method.

So don't, as a habit, write wrappers like CommitAsync for every synchronous method you have -- instead you want to make a true CommitAsync that uses the async support of the underlying libraries/frameworks (SqlCommand.ExecuteReaderAsync(), etcetera.)

If you have no choice and must use Task.Run, then the appropriate usage would look more like:

// This method is in the UI layer.
public async Task MethodAsync() {
    // Operation One:

    // Commit() is a method in the DA layer.
    await Task.Run(() => Commit());

    // Operation Two.
}