Is there a reason to prefer one of these implementations over the other

Question

In his PluralSight course Asynchronous C# 5, Jon Skeet provides this implementation for a convenience extension method called InCOmpletionOrder:

public static IEnumerable<Task<T>> InCompletionOrder<T>(this IEnumerable<Task<T>> source)
{
    var inputs = source.ToList();
    var boxes  = inputs.Select(x => new TaskCompletionSource<T>()).ToList();
    int currentIndex = -1;

    foreach (var task in inputs)
    {
        task.ContinueWith(completed =>
        {
            var nextBox = boxes[Interlocked.Increment(ref currentIndex)];
            PropagateResult(completed, nextBox);
        }, TaskContinuationOptions.ExecuteSynchronously);
    }

    return boxes.Select(box => box.Task);
}

private static void PropagateResult<T>(Task<T> completedTask,
      TaskCompletionSource<T> completionSource)
{
    switch(completedTask.Status)
    {
      case TaskStatus.Canceled:
          completionSource.TrySetCanceled();
          break;
      case TaskStatus.Faulted:
          completionSource.TrySetException(completedTask.Exception.InnerExceptions);
          break;
      case TaskStatus.RanToCompletion:
          completionSource.TrySetResult(completedTask.Result);
          break;
      default:
          throw new ArgumentException ("Task was not completed.");
    }
}

In this question, Martin Neal provides a, seemingly more elegant, implementation using yield return

public static IEnumerable<Task<T>> InCompletionOrder<T>(this IEnumerable<Task<T>> source)
{
    var tasks = source.ToList();

    while (tasks.Any())
    {
        var t = Task.WhenAny(tasks);
        yield return t.Result;
        tasks.Remove(t.Result);
    }
}

Being still somewhat new to the rigours of asynchronous programming, can anyone describe the specific concerns that might arise with Martin Neal's implementation that are properly resolved by Jon Skeet's more involved implementation

Answer 1

The second solution has a quadratic time complexity issue. The loop runs N times and each WhenAny call adds N continuations to those tasks. Don't use that code except if you know for sure that the number of tasks is very small.

The Remove call causes quadratic time complexity as well.

Also, the second piece of code is blocking. You only get back tasks as they complete. InCompletionOrder gives you those tasks immediately and they complete later.

I would regard InCompletionOrder as a library method. Put it into a utility file and it will not cause you maintenance problems. It's behavior will/can never change. I don't see code size as an issue here.