Cancellable progress form using async/await

Question

Introduction:

For a project I'm working on I used a BackGroundWorker to lift some heavy tasks away from the UI thread and report progress. For this I created some sort of modal dialog with a label and progress bar which were updated from the ProgressChanged method of the backgroundworker.

I read a very interesting article on async/await and I thought I might update my code, since the BackGroundWorker is, despite being handy, getting outdated anyway. After some more reading on this matter I found an article with code that had the backgroundworker instance inside the modal dialog form and not on the caller form.

This gave me the idea to do the same but using async/await instead. This is a first result, which is working, and I was wondering if I am doing things right. Any tips and/or improvements are much appreciated, certainly regarding scalability/extensibility and the use of async/await.

The form:

The code:

public partial class CancellableProgress : Form
{
    private readonly IEnumerable<KeyValuePair<string, Action>> _tasks;
    private CancellationTokenSource _cancelTokenSource;
    private bool _isTaskCompleted;

    public CancellableProgress(string message, Action action)
        : this(new[] { new KeyValuePair<string, Action>(message, action) })
    {
    }

    public CancellableProgress(IEnumerable<KeyValuePair<string, Action>> tasks)
    {
        InitializeComponent();
        Load += CancellableProgress_Load;
        _tasks = tasks;
    }

    private void CancellableProgress_Load(object sender, EventArgs e)
    {
        PerformTasks(_tasks);
    }

    private async void PerformTasks(IEnumerable<KeyValuePair<string, Action>> tasks)
    {
        _cancelTokenSource = new CancellationTokenSource();
        var token = _cancelTokenSource.Token;
        var progressHandler = new Progress<string>(value => MessageLabel.Text = value);
        var progress = progressHandler as IProgress<string>;

        try
        {
            await Task.Run(() =>
            {
                foreach (var task in tasks)
                {
                    if (token.IsCancellationRequested)
                    {
                        token.ThrowIfCancellationRequested();
                    }
                    progress.Report(task.Key);
                    task.Value();
                }
            }, token);
            _isTaskCompleted = true;
        }
        catch (OperationCanceledException)
        {
            MessageLabel.Text = @"Cancelled!";
        }
        catch (Exception ex)
        {
            MessageLabel.Text = ex.Message;
        }
        finally
        {
            _cancelTokenSource.Dispose();
            DialogResult = _isTaskCompleted ? DialogResult.OK : DialogResult.Cancel;
        }
    }

    private void btnCancel_Click(object sender, EventArgs e)
    {
        CancelTaskButton.Enabled = false;
        _cancelTokenSource.Cancel();
    }
}

Example usage:

This is some code on how I use the modal form.

private void TaskButton_Click(object sender, EventArgs e)
{
    var operations = new[]
    {
        new KeyValuePair<string, Action>("Operation 1", LongOperation),
        new KeyValuePair<string, Action>("Second operation", LongOperationTwo)
    };

    using (var progress = new CancellableProgress(operations))
    {
        progress.ShowDialog();
    }
}

private static void LongOperation()
{
    for (var i = 1; i <= 5; i++)
        System.Threading.Thread.Sleep(1000);
}

private static void LongOperationTwo()
{
    for (var i = 1; i <= 5; i++)
        System.Threading.Thread.Sleep(1000);
}

Answer 1

Clean code, easy to follow its meaning. Some minor remarks:

var progressHandler = new Progress<string>(value => MessageLabel.Text = value);
var progress = progressHandler as IProgress<string>;

This looks like you can just define it as

IProgress<string> progressHandler = new Progress(value => MessageLabel.Text = value);

Or just a Progress<string> type, really:

var progressHandler = new Progress(value => MessageLabel.Text = value);

Unless I'm mistaking, you don't actually need it as an interface. Note that I also removed the explicit type parameter since that should be inferred from the argument passed to it.

---

MessageLabel.Text = @"Cancelled!";

The verbatim string isn't necessary here

---

if (token.IsCancellationRequested)
{
    token.ThrowIfCancellationRequested();
}

This seems redundant to me.

It should be either this

if (token.IsCancellationRequested)
{
    throw new OperationCanceledException();
}

or this:

token.ThrowIfCancellationRequested()

This is also reinforced by the documentation

---

private async void PerformTasks()

Asynchronous methods should return Task or Task<T>, the former being for traditional void methods and the latter being for methods with T return type.

The reason for this is that a void method cannot be awaited (since it doesn't return a Task) and will in effect be a fire-and-forget kind of thing: it will start executing your asynchronous code on a separate thread and continue doing business as normal. However once the main thread reaches its end, the application will exit regardless of whether or not your asynchronous code has finished as well. This obviously leads to nasty bugs.

You won't notice this in this case because you're using a GUI which means there will always be a main thread active (as long as the application runs).

However in case you ever want to re-use this code for a console application or just in a class library where you don't know the client, I would suggest making it async Task just for good measure.

The only exception to this are asynchronous event handlers since they have to adhere to the convention that event handlers return void. This also explains why events are "dangerous" to use in asynchronous code.