How to wrap a legacy synchronous I/O bound operation with an asynchronous method?

Question

I'm still learning async/await concepts so please bear with me. Suppose there is this legacy code that defines a contract for getting data from a data source (local database in the real case):

public interface IRepository<T> {
    IList<T> GetAll();
}

In new code I have the following interface:

public interface IMyObjectRepository {
    Task<IEnumerable<MyObject>> GetAllAsync();
}

The MyObjectRepository class depends on IRepository<T> like this:

public class MyObjectRepository : IMyObjectRepository
{
    private readonly IRepository<Some.Other.Namespace.MyObject> repo_;

    private readonly IDataMapper<Some.Other.Namespace.MyObject, MyObject> mapper_;

    public BannerRepository(IRepository<Some.Other.Namespace.MyObject> repo, IDataMapper<Some.Other.Namespace.MyObject, MyObject> mapper)
    {
        repo_ = repo;
        mapper_ = mapper;
    }
}

How do I implement the IMyObjectRepository.GetAllAsync() method in such a way that it makes sense in the context of asynchronous programming? Getting data from some local data source is an I/O bound operation, so the way I did it is:

public async Task<IEnumerable<MyObject>> GetAllAsync()
{
    var tcs = new TaskCompletionSource<IEnumerable<MyObject>>();

    try
    {
        GetAll(objects =>
        {
            var result = new List<MyObject>();

            foreach (var o in objects)
            {
                result.Add(mapper_.Map(o));
            }

            tcs.SetResult(result);
        });
    }
    catch (Exception e)
    {
        tcs.SetException(e);
    }

    return await tcs.Task;
}

private void GetAll(Action<IEnumerable<Some.Other.Namespace.MyObject>> handle)
{
    IEnumerable<Some.Other.Namespace.MyObject> objects = repo_.GetAll<Some.Other.Namespace.MyObject>();

    if (objects is null)
    {
        objects = Enumerable.Empty<Some.Other.Namespace.MyObject>();
    }

    handle(objects);
}

Does this make any sense? I did not want to use Task.Run() because, the way I understand it, that wastes a thread for nothing in the case of an I/O bound operation which makes sense.

Answer 1

The approach in the example is a valid one. I would however be careful to only use it when absolutely needed, since users might get very confused when calling the "async" method, only for it to block. In that sense it might be preferable to use Task.Run() since that will not block. But it does require the called code to be threadsafe.

In your example there is no point in using async, just remove that and the await should give the exact same result, since you are returning a task anyway.

Using Task.FromResult is another easy alternative. But it will not be quite identical if exception occurs. Using a TaskCompletionSource and .SetException will make exceptions occur as a property of the Task.Exception rather than raised from the method call itself. If the task is awaited it should not matter, since both cases should be handled by a try/catch. But there are other ways to handle exceptions for tasks, so you need to be aware how it is used, and provide appropriate documentation/comments to describe the behavior.