Would a Task.Convert extension method

问题:

I'm writing client libraries for Google Cloud APIs which have a fairly common pattern for async helper overloads:

• Do some brief synchronous work to set up a request
• Make an asynchronous request
• Transform the result in a simple way

Currently we're using async methods for that, but:

• Transforming the result of await ends up being annoying in terms of precedence - we end up needing (await foo.Bar().ConfigureAwait(false)).TransformToBaz() and the brackets are annoying. Using two statements improves readability, but means we can't use an expression-bodied method.
• We occasionally forget ConfigureAwait(false) - this is solvable with tooling to some extent, but it's still a bit of a smell

Task<TResult>.ContinueWith sounds like a good idea, but I've read Stephen Cleary's blog post recommending against it, and the reasons seem sound. We're considering adding an extension method for Task<T> like this:

Potential extension method

public static async Task<TResult> Convert<TSource, TResult>(
    this Task<TSource> task, Func<TSource, TResult> projection)
{
    var result = await task.ConfigureAwait(false);
    return projection(result);
}

We can then call this from a synchronous method really simply, e.g.

public async Task<Bar> BarAsync()
{
    var fooRequest = BuildFooRequest();
    return FooAsync(fooRequest).Convert(foo => new Bar(foo));
}

or even:

public Task<Bar> BarAsync() =>
    FooAsync(BuildFooRequest()).Convert(foo => new Bar(foo));

It seems so simple and useful that I'm slightly surprised there isn't something already available.

As an example of where I'd use this to make an expression-bodied method work, in the Google.Cloud.Translation.V2 code I have two methods to translate plain text: one takes a single string and one takes multiple strings. The three options for the single-string version are (simplified somewhat in terms of parameters):

Regular async method

public async Task<TranslationResult> TranslateTextAsync(
    string text, string targetLanguage)
{
    GaxPreconditions.CheckNotNull(text, nameof(text));
    var results = await TranslateTextAsync(new[] { text }, targetLanguage).ConfigureAwait(false);
    return results[0];
}

Expression-bodied async method

public async Task<TranslationResult> TranslateTextAsync(
    string text, string targetLanguage) =>
    (await TranslateTextAsync(new[] { GaxPreconditions.CheckNotNull(text, nameof(text)) }, targetLanguage)
        .ConfigureAwait(false))[0];

Expression-bodied sync method using Convert

public Task<TranslationResult> TranslateTextAsync(
    string text, string targetLanguage) =>
    TranslateTextAsync(new[] { GaxPreconditions.CheckNotNull(text, nameof(text)) }, targetLanguage)
        .Convert(results => results[0]);

I personally prefer the last of these.

I'm aware that this changes the timing of the validation - in the final example, passing a null value for text will immediately throw an ArgumentNullException whereas passing a null value for targetLanguage will return a faulted task (because TranslateTextAsync will fail asynchronously). That's a difference I'm willing to accept.

Are there differences in scheduling or performance that I should be aware of? (We're still constructing two state machines, because the Convert method will create one. Using Task.ContineWith would avoid that, but has all the problems mentioned in the blog post. The Convert method could potentially be changed to use ContinueWith carefully.)

(I'm somewhat tempted to post this on CodeReview, but I suspect the information in the answers will be more generally useful beyond whether this is specifically a good idea. If others disagree, I'm happy to move it.)

回答1:

Transforming the result of await ends up being annoying in terms of precedence

I generally prefer to introduce a local var, but as you noted, that prevents expression-bodied methods.

We occasionally forget ConfigureAwait(false) - this is solvable with tooling to some extent

Since you're working on a library and should use ConfigureAwait(false) everywhere, it may be worthwhile to use a code analyzer that enforces ConfigureAwait usage. There's a ReSharper plugin and a VS plugin that do this. I haven't tried them myself, though.

Task<TResult>.ContinueWith sounds like a good idea, but I've read Stephen Cleary's blog post recommending against it, and the reasons seem sound.

If you used ContinueWith, you'd have to explicitly specify TaskScheduler.Default (this is the ContinueWith equivalent of ConfigureAwait(false)), and also consider adding flags such as DenyChildAttach. IMO it's harder to remember how to use ContinueWith correctly than it is to remember ConfigureAwait(false).

On the other hand, while ContinueWith is a low-level, dangerous method, if you use it correctly then it can give you minor performance improvements. In particular, using the state parameter can save you a delegate allocation. This is the approach commonly taken by the TPL and other Microsoft libraries, but IMO it lowers the maintainability too much for most libraries.

It seems so simple and useful that I'm slightly surprised there isn't something already available.

The Convert method you suggest has existed informally as Then. Stephen doesn't say so, but I assume that the name Then is from the JavaScript world, where promises are the task equivalent (they are both Futures).

On a side note, Stephen's blog post takes this concept to an interesting conclusion. Convert/Then is the bind for the Future monad, so it can be used to implement LINQ-over-futures. Stephen Toub has also published code for this (rather dated at this point, but interesting).

I have thought a few times about adding Then to my AsyncEx library, but each time it didn't make the cut because it's pretty much the same as just await. Its only benefit is solving the precedence problem by allowing method chaining. I assume it doesn't exist in the framework for the same reason.

That said, there's certainly nothing wrong with implementing your own Convert method. Doing so will avoid the parenthesis / extra local variable and allow for expression-bodied methods.

I'm aware that this changes the timing of the validation

This is one of the reasons that I'm wary of eliding async/await (my blog post goes into more reasons).

In this case, I think it's fine either way, since the "brief synchronous work to set up a request" is a preconditions check, and IMO it doesn't matter where boneheaded exceptions are thrown (because they shouldn't be caught anyway).

If the "brief synchronous work" was more complex - if it was something that could throw, or could reasonably throw after someone refactors it a year from now - then I would use async/await. You could still use Convert to avoid the precedence issue:

public async Task<TranslationResult> TranslateTextAsync(string text, string targetLanguage) =>
  await TranslateTextAsync(SomthingThatCanThrow(text), targetLanguage)
  .Convert(results => results[0])
  .ConfigureAwait(false);