Task with feedback dialog helper class

Question

I'm currently working in a Windows.Forms project where its very common that I need to show a responsive feedback form while a task runs. These tasks take typically around 5 to 10 seconds to complete and the user can't do anything else until the task has successfully finished or failed.

To keep the UI responsive I've cooked up a little helper class. I'm pretty new to tasks and the async programming environment so I am not sure if I'm overkilling things here or going off in a completely wrong direction:

This is the small helper class:

internal sealed class TaskManagerWithFeedbackDialog<TController> where TController : IViewController, new() 
{
    public static Task<TReturn> RunTask<TReturn>(Func<TReturn> func)
    {
        if (func == null)
        {
            throw new ArgumentNullException("func");
        }

        var manager = new _AsyncTaskManager<TReturn>(func);

        using (var view = new TController())
        {
            var task = manager.Start(view);
            view.ShowViewDialog();
            return task;
        }
    }

    private class _AsyncTaskManager<T>
    {
        private readonly Func<T> taskFunc;

        public _AsyncTaskManager(Func<T> func)
        {
            taskFunc = func;
        }

        public Task<T> Start(IViewController feedBackView)
        {
            var task = Task.Factory.StartNew(taskFunc);

            task.ContinueWith(t =>
            {
                feedBackView.CloseView();
            }, TaskContinuationOptions.OnlyOnFaulted);

            task.ContinueWith(t =>
            {
                feedBackView.CloseView();
                return t.Result;
            }, TaskContinuationOptions.NotOnFaulted);

            return task;
        }
    }
}

Where the implementation of IViewController.CloseView() would look like this:

public virtual void CloseView()
{
    if (View.InvokeRequired)
    {
        View.Invoke(new Action(View.Close));
    }
    else
    {
        View.Close();
    }
}

Another issue I am not sure about is where is the best place to handle ArregateException?. Right now I don't handle anything as I don't need to clean up anything if a task fails. I simply check the returned task for any errors. Is this the right thing to do? Do async tasks always fail 'silently'? If I needed to clean up, where would it be? Inside .ContinueWith( .... , TaskContinuationOptions.OnlyOnFaulted)?

Answer 1

Few minor things:

• If TaskManagerWithFeedbackDialog is not a nested class then internal is redundant and can be dropped.

• TaskManagerWithFeedbackDialog has - so far - just one single static method. Until you will add more methods make it a static class. No need to create any instance.

• TController instance is disposed just after task started (see it's wrapped in using). When you will invoke CloseView() you will call it on an already disposed object. Is it just a factory? Call it *Factory instead of *Controller. Is it a controller? Don't dispose it immediately but when you close the view (and check why your Dispose() method does nothing).

• Why RunTask() instead of StartNew() or - better - Run() as Task and TaskFactory methods? I'd favor consistency with framework classes.

• Is there a specific reason you're using Task.Factory.StartNew() instead of Task.Run()? EDIT: OK, you're with .NET 4.0.

• _AsyncTaskManager (which BTW shouldn't be prefixed, it's optional for private fields and to avoid for types) is superfluous. It's just one method, it has no state and it's not persisted or passed around. You don't need a class but a simple private method.

• Task has a fluid interface, you can concatenate method calls return Task.Run(...).ContinueWith(...).ContinueWith(...).

• In CloseView() you wait for dispatched action to terminate but you don't actually need to wait for UI to push thread back to the pool, in WinForms you may simply use BeginInvoke() because it doesn't need EndInvoke() to be called.

Ideally you wouldn't even need continuation tasks:

Task.Factory.StartNew(() => {
    try
    {
        return taskFunc();
    }
    finally
    {
        feedBackView.CloseView();
    }
});

About exception management: you're doing right, if it's the way you want to handle errors. You close your view but AggregateException goes unnoticed. Here you may want to log such errors or to propagate them to UI thread (for example) again using BeginInvoke. See MSDN for more details.

Answer 2

Updated Here is a little bit of cleanup. Now you can get back reference to Task<T> before blocking starts: TaskWithFeedback.Run() is a non blocking call. It also works with Form.Show() and Form.ShowModel() in the view.

class TaskWithFeedback<TViewController> 
    where TViewController : IViewController, new()
{
    public static async Task<T> Run<T>(Func<T> function)
    {
        var context = TaskScheduler.FromCurrentSynchronizationContext();
        var view = new TViewController();
        var opening = new Task(view.ShowViewDialog);
        opening.Start(context);
        var task = Task.Run(function);
        var closing = task
            .ContinueWith(t => view.CloseViewDialog(), context);                

        try
        {                             
            return await task;
        }
        finally
        {
            await opening;
            await closing;
        }
    }
}

Example:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }

    private async void button1_Click(object sender, EventArgs e)
    {
        Text = await TaskWithFeedback<Form2>.Run(() =>
            {
                Thread.Sleep(3000);
                return "Hello!";
            });            
    }   
}

class Form2 : Form, IViewController
{
    public void ShowViewDialog() => ShowDialog();
    public void CloseViewDialog() => Close();
}

Answer 3

I'm not very familiar with C# or Windows.Forms for that matter. But I'll try to help you in regards to:

Another issue I am not sure about is where is the best place to handle ArregateException?

Looking at the msdn docs you can deal with the task result in two ways:

• inside the ContinueWith(...) closure by specifying the correct TaskContinuationOptions (in your case, for dealing with any exceptions thrown, would be the OnlyOnFaulted enum option;

• try to access the Result property of the Task instance that you return. If the operation was unsuccessful, this will throw the underlying exception, which you can then handle.

Accessing the Result property later would make sense if you waited for the task to finish. But I guess that's not your use case (and the main reason you decided to do this in a Task). So I would just go with the ContinueWith(...).

One final thing to bear in mind, (and I gladly welcome any C# developer to correct me on this one) is that a task is just a unit of work. A process if you want to think of it that way.

You give an operation to it (in this case the Func) and it will run it whenever it can. The ContinueWith() methods are just continuations, things that will run after the task is finished (being it an error, success, or cancel).

The responsibility of your TaskManagerWithFeedbackDialog is to run whatever operation you give it and deal with the success or failure of that operation. I hope this helps refactoring your original code.

Remember, keep it simple!