views:

68

answers:

2

I'm trying to figure out the best way to handle a backgroundworker that is triggered off radio button clicks. I created a very simple form with 3 radio buttons and a label. Each of the radio buttons share the same event radioButton_CheckedChanged. If the event completes then I update the label to "Complete". If you click another radio button before the event completes then update label to Cancelled. Below is the code i have written in this quick example. Although the application tends to run as expected my concern is the use of Application.DoEvents. What are my alternatives to this. For obvious reasons i can't sleep while IsBusy. Am I going about this all wrong or is there a better way to do this? Thanks, poco

private void radioButton_CheckedChanged(object sender, EventArgs e)
{
  RadioButton rb = sender as RadioButton;
            if (rb.Checked)
            {
                if (backgroundWorker1.IsBusy)
                {
                    backgroundWorker1.CancelAsync();
                    while (backgroundWorker1.IsBusy)
                        Application.DoEvents();
                }

                backgroundWorker1.RunWorkerAsync();
            }
        }

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
        {
            BackgroundWorker worker = sender as BackgroundWorker;
            for (int i = 0; i < 100 && !worker.CancellationPending; ++i)
                Thread.Sleep(1);

            if (worker.CancellationPending)
            {
                e.Cancel = true;
                return;
            }
        }

        private void backgroundWorker1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
        {
            if (e.Cancelled)
                label1.Text = "Canceled";
            else
                label1.Text = "Complete";
        }
+3  A: 

You should move the code that must run when the BackgroundWorker completes into the RunWorkerCompleted hander. In pseudo-code:

private void radioButton_CheckedChanged(object sender, EventArgs e)
{
    // ...

    if (backgroundWorker1.IsBusy)
    {
        backgroundWorker1.CancelAsync();
        addJobToQueue();   // Don't wait here, just store what needs to be executed.
    } else {
        backgroundWorker1.RunWorkerAsync();
    } 
}

private void backgroundWorker1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    if (e.Cancelled) {
        label1.Text = "Canceled";
    }
    else {
        label1.Text = "Complete";
    }

    // We've finished! See if there is more to do...
    if (thereIsAnotherJobInTheQueue())
    {
         startAnotherBackgroundWorkerTask();
    }
}
Mark Byers
Ok, I follow this, I'll go test it out. I would assume that JobQueue would be on the GUI thread so do i ever run the risk of crossthreading? The only time i would see changing the jobQueue would be in the rb_checkchanged and in the bw_runcompleted which should be back on the main thread right?
poco
A: 

DoEvents is not supposed to be taken so casually. There are better ways. One of the very nice ones is described here in SO. This answer is probably best for you.

Hence your solution becomes:

private AutoResetEvent _resetEvent = new AutoResetEvent(false);

private void radioButton_CheckedChanged(object sender, EventArgs e)
{
    RadioButton rb = sender as RadioButton;
    if (rb.Checked)
    {
        if (backgroundWorker1.IsBusy)
        {
            backgroundWorker1.CancelAsync();
            _resetEvent.WaitOne(); // will block until _resetEvent.Set() call made
        }

        backgroundWorker1.RunWorkerAsync();
    }
}

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
    BackgroundWorker worker = sender as BackgroundWorker;
    for (int i = 0; i < 100 && !worker.CancellationPending; ++i)
        Thread.Sleep(1);

    if (worker.CancellationPending)
    {
        e.Cancel = true;
    }
    _resetEvent.Set();
}
Nayan
This results in -> This BackgroundWorker is currently busy and cannot run multiple tasks concurrently when calling backgroundWorker1.RunWorkerAsync()
poco
True..........!
Nayan