3
\$\begingroup\$

I'm attempting to write a Progress Dialog that I can instantiate, show and then pass a task to complete to. I'm pretty new to Task Based Patterns, so please bear with me.

Ideally my goal is to be able to show the progress of a task in a popup window, similar to the VS "Background Operation in Progress" dialog that you may know. Hopefully this can be fairly generic, or at very least define some sort of pattern that I can reuse.

What I have is below. Any suggestions are welcome!

private CancellationTokenSource _CancellationTokenSource;

        /// <summary>
        /// Constructor.
        /// </summary>
        public frmThreadProgress()
        {
            InitializeComponent();
        }

        public static frmThreadProgress ShowProgress()
        {
            frmThreadProgress f = new frmThreadProgress();
            f.Show();
            f.BringToFront();
            return f;
        }

        public void Cancel()
        {
            // Cancel the background task.
            _CancellationTokenSource.Cancel();

            // The UI will be updated by the cancellation handler.
        }

        public delegate Task<TResult> GenericTask<TResult, in TInput>(
            TInput input, IProgress<int> progress, CancellationToken cancel);


        /// <summary>
        /// Run a Task Asynchronously and display its progress
        /// </summary>
        /// <typeparam name="TResult"></typeparam>
        /// <typeparam name="TInput"></typeparam>
        /// <param name="generictask"></param>
        /// <param name="input"></param>
        /// <param name="description"></param>
        /// <returns></returns>
        public Task<TResult> RunAsync<TInput, TResult>(GenericTask<TResult, TInput> generictask, TInput input, string description)
        {
            TaskIsRunning();
            Information = description;
            _CancellationTokenSource = new CancellationTokenSource();
            var cancellationToken = _CancellationTokenSource.Token;
            var progressReporter = new ProgressReporter();
            IProgress<int> onProgress = new Progress<int>((x) => pb.Position = x);
            Task<TResult> t = generictask(input, onProgress, cancellationToken);
            //configure completion handlers
            progressReporter.RegisterCancelledHandler(t, TaskIsCancelled);
            progressReporter.RegisterSucceededHandler(t, TaskIsComplete);
            progressReporter.RegisterFaultedHandler(t, TaskErrored);
            return t;
        }

        /// <summary>
        /// Iterate a List of items and apply a function to produce a list of results
        /// Reports progress of the task
        /// </summary>
        /// <typeparam name="TResult"></typeparam>
        /// <typeparam name="TInput"></typeparam>
        /// <param name="input"></param>
        /// <param name="operation"></param>
        /// <param name="description"></param>
        /// <returns></returns>
        public Task<IList<TResult>> IterateAsync<TInput, TResult>(IList<TInput> input, Func<TInput, TResult> operation, string description)
        {
            TaskIsRunning();
            Information = description;
            _CancellationTokenSource = new CancellationTokenSource();
            var cancellationToken = _CancellationTokenSource.Token;
            var progressReporter = new ProgressReporter();
            //delegate to update the progress
            IProgress<int> onProgress = new Progress<int>((x) => pb.Position = x);
            //Start the task
            Task<IList<TResult>> t = Task.Factory.StartNew(() =>
            {
                IList<TResult> output = new List<TResult>();
                for (int i = 0; i < input.Count; i++)
                {
                    cancellationToken.ThrowIfCancellationRequested();
                    TResult r = operation(input[i]);
                    output.Add(r);
                    onProgress.Report(100 * i / input.Count);
                }
                return output;
            }, cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Default);
            //Register completion/error/cancel handlers
            progressReporter.RegisterCancelledHandler(t, TaskIsCancelled);
            progressReporter.RegisterSucceededHandler(t, TaskIsComplete);
            progressReporter.RegisterFaultedHandler(t, TaskErrored);
            return t;
        }

        private void TaskIsRunning()
        {
            // Update UI to reflect background task.
            btnOk.Enabled = false;
            btnCancel.Enabled = true;
        }

        private void TaskIsComplete()
        {
            // Reset UI.
            pb.Position = 100;
            btnOk.Enabled = true;
            btnCancel.Enabled = false;
        }

        private void TaskIsCancelled()
        {
            btnOk.Enabled = true;
            btnCancel.Enabled = false;
            Information = "User cancelled the operation!";
        }

        private void TaskErrored(Exception e)
        {
            btnOk.Enabled = true;
            btnCancel.Enabled = false;
            Information = "Error";
            frmMessage.Show("Error running task", e);
        }

        ///// <summary>
        ///// Information Text
        ///// </summary>
        public string Information
        {
            set { lblInformation.AsyncUpdate(() => lblInformation.Text = value); }
        }

        private void buttonCancel_Click(object sender, EventArgs e)
        {
            //notify the background worker we want to cancel
            Cancel();
        }

        private void btnOk_Click(object sender, EventArgs e)
        {
            DialogResult = DialogResult.OK;
            Close();
        }
\$\endgroup\$
1
\$\begingroup\$

I think there is a design problem here.

If your window is truly generic, it need not know how the tasks are being launched/queued. I don't think it's a good idea to schedule operations inside Task.Factory.StartNew—your window has no idea what kind of operations they might be.

For example, these operations might interact with UI—or just with shared non-threadsafe variables—and it wouldn't be immediately obvious to the client code that operations run on another thread. Moreover, the client code might not want to schedule them on another thread (consider waiting for several asynchronous operations already represented as Tasks). You're also constraining the operations to run sequentially (instead of running e.g. in parallel, which can be mapped to progress bar just as well).

I'm also very sceptical about the whole TInput/TResult generic thing here; you aren't using these types, are you? This seems way overcomplicated for a fairly simple task. What if operation needs more than one parameter? Returns no result?

Instead, I think you should let window accept an array of non-generic Tasks, track them and that's it. No TInput/TResult. Of course you may want to keep IProgress—in this case, I'd suggest packing it in a struct together with Task. As in

public struct TaskWithProgress {
    public Task Task { get; set; }
    public IProgress<double> Progress { get; set; }
}

public Task ObserveAsync(TaskWithProgress[] tasks, string description)    

After all, your window only needs to know how may tasks there are and how many of them have completed. It doesn't need to know any parameters or results. The client code can trivially build an array of results with Task.WhenAll if it needs to.

It would then be client's code responsibility to schedule tasks (via Task.Factory.StartNew or any other mechanism).

As an aside, GenericTask is a bad name because it assumes some relation to Task (which you unfortunately do not use—but you should!)

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks for the input. This is true that at the moment, its not very generic. I frequently find myself needing to run a block of code that takes a while to run, and I simply would like a fairly simple framework to be able to show a progress dialog, and provide feedback on progress. Could you suggest a useful mechanism/pattern to allow a task to supply the progress information? \$\endgroup\$ – Simon Jan 5 '14 at 12:42
  • \$\begingroup\$ @Simon: I missed this bit—and now amended the answer. I'd use IProgress too, but with double (from 0 to 1) instead of int to avoid divisions and multiplications by 100 everywhere. \$\endgroup\$ – Dan Jan 5 '14 at 13:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.