.ContinueWith() and object state

Question

I'm waffling between which is better (in terms of aesthetics, idiomatic-ness, and performance):

public async Task<string> DecryptAsync(string encrypted)
{
    SymmetricAlgorithm aes = this.GetAes();

    return await this.DecryptAsync(aes, encrypted).ContinueWith(
        (decryptTask, objectState) =>
        {
            (objectState as IDisposable)?.Dispose();
            return decryptTask.Result;
        },
        aes);
}

or

public async Task<string> DecryptAsync(string encrypted)
{
    SymmetricAlgorithm aes = this.GetAes();

    return await this.DecryptAsync(aes, encrypted).ContinueWith(decryptTask =>
    {
        aes.Dispose();
        return decryptTask.Result;
    });
}

The main difference being that the second one captures the aes variable in the lambda while the first one passes it as a parameter then casts it to the appropriate type.

A third consideration, inspired by Oxald and Servy:

public async Task<string> DecryptAsync(string encrypted)
{
    using (SymmetricAlgorithm aes = this.GetAes())
    {
        return await this.DecryptAsync(aes, encrypted);
    }
}

I've always figured that if casting can be avoided, then avoid it. — Brandon Miller, Mar 21 '18 at 15:05
I wonder if "using(SymmetricAlgorithm aes = this.GetAes())" is a better working solution, since "using" statement already works fine along with IEnumerable/yield pattern. — Oxald, Mar 21 '18 at 15:12
Why did you tag the question with [tag:async-await] if you aren't actually using `await`? And more importantly, why aren't you using `await`? — Servy, Mar 21 '18 at 15:18
@Oxald yours is an excellent question - I came to this design because of `HttpClient`'s `HttpResponseMessage` `Content.ReadAsStreamAsync` would be invalid if the `HttpResponseMessage` was wrapped in a `using` as it would `Dispose()` before reading from the steam, making it invalid. The `ContinueWith()` pattern became part of the solution there. — Jesse C. Slicer, Mar 21 '18 at 15:19
@Servy sorry, I elided it at that level due to a ReSharper hint. Put them back in for now. — Jesse C. Slicer, Mar 21 '18 at 15:20
@JesseC.Slicer There unnecessary when you use `ContinueWIth` (incorrectly) to try to run code after the task finishes. They're not unnecessary when you use `await` to run code after a task finishes, with the proper semantics. — Servy, Mar 21 '18 at 15:22
@Servy allow me to add a third alternative then for consideration. — Jesse C. Slicer, Mar 21 '18 at 15:25
@JesseC.Slicer : did you test your third approach? does it have the same side effects described in your previous comment (with HttpClient) ? — Oxald, Mar 21 '18 at 15:36
Now just declare the string before the using block, assign it inside the using block and return it after the using block and you're set. — Jesse de Wit, Mar 21 '18 at 15:36
@Oxald working up a full test now. I hope this is the solution as it's the cleanest looking of the bunch. — Jesse C. Slicer, Mar 21 '18 at 15:47
@JesseSlicer, it is not necessary to declare the string outside the using block, but it does avoid confusion about when the object is disposed. No performance cost + more readable = better. — Jesse de Wit, Mar 22 '18 at 09:32

score 1 · Accepted Answer · answered Mar 26 '18 at 13:40

Consider using await instead of ContinueWith. the result of await equals Task.Result. The Disposing of aes can be done using a using statement:

public async Task<string> DecryptAsync(string encrypted)
{
    using (SymmetricAlgorithm aes = this.GetAes())
    {
        string decryptedText = await this.DecryptAsync(aes, encrypted);
        return decryptedText;
    };
}

.ContinueWith() and object state

1 Answers1