Question

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";
        }
Was it helpful?

Solution

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();
    }
}

OTHER TIPS

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();
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top