10
\$\begingroup\$

I have finished a program, and it does what I want it to do, but I feel I am "doing it wrong", even though it's seemingly efficient enough. I have prepared a small example of what I feel I am handling wrong with the backgroundworker class and would like to see if I could have handled this any more cleanly.

First, I have a form with 1 button, 1 statusstrip, 1 toolstripprogressbar, and 1 toolstriplabel. I update the statusstrio items while the backgroundworker is running, as well as show messageboxes across classes. See below.

Form1.cs

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

        private void prepareToRun()
        {
            if (!backgroundWorker1.IsBusy)
            {
                backgroundWorker1 = new BackgroundWorker();
                backgroundWorker1.DoWork += delegate
                {
                    Looping.loop(statusStrip1);
                };
                backgroundWorker1.RunWorkerAsync();
            }
        }

        private void button1_Click(object sender, EventArgs e)
        {
            prepareToRun();
        }

    }
}

Looping.cs

namespace StatusStripTest
{
    public class Looping
    {
        public static void loop(StatusStrip strip)
        {
            Form1 Form1 = new Form1();
            string alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
            while (true)
            {
                MessageBox.Show("Time to loop");
                foreach (char c in alphabet)
                {
                    StripHandler.UpdateProgress(strip, alphabet.IndexOf(c), alphabet.Length);
                    StripHandler.UpdateStatus(strip, c.ToString());
                }
            }

        }
    }

}

StripHandler.cs

namespace StatusStripTest
{
    public class StripHandler
    {
        public static void UpdateStatus(StatusStrip ss, String Status)
        {
            ss.Invoke((MethodInvoker)delegate
            {
                if ((ss.Items[1] as ToolStripStatusLabel) != null)
                {
                    ss.Items[1].Text = Status;
                }
            });
        }

        public static void UpdateProgress(StatusStrip ss, long position, long len)
        {
            ss.Invoke((MethodInvoker)delegate
            {
                if ((ss.Items[0] as ToolStripProgressBar) != null)
                {
                    ToolStripProgressBar tspb = (ToolStripProgressBar)ss.Items[0];
                    tspb.Minimum = 0;
                    tspb.Maximum = (int)len;
                    tspb.Value = (int)position;
                }
            });
        }
    }
}

Am I passing the statusstrip incorrectly?

\$\endgroup\$
  • \$\begingroup\$ Your thing with the alphabet, what is it supposed to model? Progression of a single process? \$\endgroup\$ – Laurent LA RIZZA Jan 21 '14 at 12:10
  • \$\begingroup\$ Yes, it was a model. The 'Looping.Loop' was supposed to represent one of many different classes that needed calling depending on which button is pressed in my main program. I didn't make that clear at all, however. \$\endgroup\$ – MrFreeman Jan 22 '14 at 18:38
6
\$\begingroup\$

Here is a technique to do everything your three classes do under one file and one class name. Konrad's code makes good points, but the code below shows how to accomplish your tasks.

Since you have no need to change your BackgroundWorker's event handlers, code them up in the Form's constructor:

private const string alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";

public Form1() {
  InitializeComponent();
  backgroundWorker1 = new BackgroundWorker();
  backgroundWorker1.WorkerReportsProgress = true;
  backgroundWorker1.WorkerSupportsCancellation = true;
  backgroundWorker1.DoWork += new DoWorkEventHandler(backgroundWorker_DoWork);
  backgroundWorker1.ProgressChanged += new ProgressChangedEventHandler(backgroundWorker_ProgressChanged);
}

I also defined alphabet as a constant, since it will never change.

Now, you need to know what backgroundWorker_DoWork and backgroundWorker_ProgressChanged are.

This version is closest to the code you posted, using a foreach loop on the alphabet string. Every time you access the next character, that character's index has to be extracted and the length of the fixed array has to be determined again:

private void backgroundWorker_DoWork_obsolete(object sender, DoWorkEventArgs e) {
  var obj = (BackgroundWorker)sender;
  while (!obj.CancellationPending) {
    foreach (char c in alphabet) {
      float calc = ((float)alphabet.IndexOf(c) / alphabet.Length) * 100;
      obj.ReportProgress(Convert.ToInt32(calc));
    }
  }
}

The modified version below is going to create the string using a value that is supplied as the argument (so, now this code can be used for other string values). Since your code basically just walks down the length of the string of characters, I modified this version to use a more efficient for(;;) loop (more efficient because it does not have to extract the index or calculate the string length every time):

private void backgroundWorker_DoWork(object sender, DoWorkEventArgs e) {
  var obj = (BackgroundWorker)sender;
  string alphas = e.Argument.ToString();
  int len = alphas.Length;
  while (!obj.CancellationPending) {
    for (int i = 0; i < len; i++) {
      float calc = ((float)i / len) * 100;
      obj.ReportProgress(Convert.ToInt32(calc));
    }
  }
}

Notice in both examples that I use while (!obj.CancellationPending) instead of while(true). Now, if you wanted to include a Cancel button on your form, you could simply wire it up like this:

private void cancel_Click(object sender, EventArgs e) {
  backgroundWorker1.CancelAsync();
}

This would halt your BackgroundWorker loop.

Now, the backgroundWorker_ProgressChanged routine is simple:

private void backgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e) {
  toolstripprogressbar.Value = e.ProgressPercentage;
}

To execute this code, just be sure to initialize your toolstripprogressbar before calling RunWorkerAsync.

private void prepareToRun() {
  if (!backgroundWorker1.IsBusy) {
    toolstripprogressbar.Minimum = 0;
    toolstripprogressbar.Value = 0;
    toolstripprogressbar.Maximum = alphabet.Length;
    backgroundWorker1.RunWorkerAsync(alphabet);
  }
}

private void button1_Click(object sender, EventArgs e) {
  prepareToRun();
}

A few extra notes:

The BackgroundWorker.ReportProgress is overloaded to accept an object userState and the BackgroundWorker.ProgressChangedEventArgs contains this UserState object. So, if you wanted to pass the actual item to some Label control called lblProgress, that is possible:

private void backgroundWorker_DoWork2(object sender, DoWorkEventArgs e) {
  var obj = (BackgroundWorker)sender;
  string alphas = e.Argument.ToString();
  int len = alphas.Length;
  while (!obj.CancellationPending) {
    foreach (char c in alphas ) {
      float calc = ((float)alphas .IndexOf(c) / len ) * 100;
      obj.ReportProgress(Convert.ToInt32(calc), c);
    }
  }
}

private void backgroundWorker_ProgressChanged2(object sender, ProgressChangedEventArgs e) {
  toolstripprogressbar.Value = e.ProgressPercentage;
  lblProgress.Text = e.UserState.ToString();
}

If you wanted to get fancy, wire up the BackgroundWorker.RunWorkerCompletedEventHandler:

  backgroundWorker1.RunWorkerCompleted += new RunWorkerCompletedEventHandler(backgroundWorker_RunWorkerCompleted);

...add this little piece of code to hide your Progress Bar and enable your Cancel Button:

private void backgroundWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e) {
  toolstripprogressbar.Visible = false;
  btnCancel.Enabled = true;
}

...and modify your prepareToRun() statement:

private void prepareToRun() {
  if (!backgroundWorker1.IsBusy) {
    toolstripprogressbar.Minimum = 0;
    toolstripprogressbar.Value = 0;
    toolstripprogressbar.Maximum = alphabet.Length;
    backgroundWorker1.RunWorkerAsync(alphabet);
    if (backgroundWorker1.IsBusy) {
      toolstripprogressbar.Visible = true;
      btnCancel.Enabled = false;
    }
  }
}

Can you tell I have used a lot of BackgroundWorkers?

UPDATE:

Another note, based on the comment you provided. The DoWorkEventArgs variable e has an object variable called Argument that corresponds to whatever input you supply in backgroundWorker1.RunWorkerAsync(any_object).

That means any_object can be a class like this:

class MyParameters {
  public int Minimum { get; set; }
  public int Index { get; set; }
  public int Maximum { get; set; }
  public string TextIn { get; set; }
  public string TextOut { get; set; }
}

To use it, simply cast it back to what you passed in:

private const int BG_MSG_1 = 1;

private void backgroundWorker_DoWork3(object sender, DoWorkEventArgs e) {
  var obj = (BackgroundWorker)sender;
  var params = (MyParameters)e.Argument;
  params.Minimum = 0;
  params.Index = 0;
  params.Maximum = params.TextIn.Length;
  while (!obj.CancellationPending) {
    foreach (char c in params.TextIn ) {
      params.Index++;
      params.TextOut = string.Format("Processed {0}", c);
      obj.ReportProgress(BG_MSG_1, params);
    }
  }
}

Notice I have re-used the ReportProgress's int variable of ProgressPercentage as a way to provide further/deeper options within the ProgressChanged Event Handler.

Now, you can do any calculations you need back in your main thread by extracting your MyParameters out of the UserState:

private void backgroundWorker_ProgressChanged3(object sender, ProgressChangedEventArgs e) {
  if (e.ProgressPercentage == BG_MSG_1) {
    var params = (MyParameters)e.UserState;
    float calc = ((float)params.Index / params.Maximum ) * 100;
    toolstripprogressbar.Value =  Convert.ToInt32(calc)
    lblProgress.Text = params.TextOut;
  }
}

Now, you are only limited by how complex you want to make whatever class you want to pass back and forth ...or you could create a class for passing data in and another class for passing data out. Really, it's all up to you.

\$\endgroup\$
  • \$\begingroup\$ Wow! I am stunned at the absolutely phenomenal explanation you have provided! It's much clearer and concise than any other documentation I have read regarding the handling of the statusBar and backgroundworkers. However, it looks as though the code limits my status updating to just the progress done within the backgroundworker's dowork event. In the code of my main program, my "Looping.loop" is actually a fairly decent chunk of code which also references other classes where progress I wish to report also happens. It also looks here that I must update the label and bar simultaneously? \$\endgroup\$ – MrFreeman Jan 22 '14 at 12:46
  • \$\begingroup\$ Not really. Instead of passing back that single char in the obj.ReportProgress(Convert.ToInt32(calc), c) line, you can pass ANY object you choose (like a class or structure of all your data), then cast it back in the ProgressChanged event. I'm late for work, though. Gotta go! \$\endgroup\$ – jp2code Jan 22 '14 at 13:17
  • \$\begingroup\$ OK, I added an Update section towards the bottom without altering what I already stated up top. \$\endgroup\$ – jp2code Jan 22 '14 at 16:32
8
\$\begingroup\$

In general, passing controls to the BackgroundWorker should not be necessary at all. Think about it as it simply has some job to do and that's all. However, it can of course report about progress of its job. Those two ideas were taken into account when designing BackgroundWorker class and are reflected by DoWork and ProgressChanged events respectively. There is also a dedicated BackgroundWorker.ReportProgress method which raises ProgressChanged event.

This event should be handled on GUI level so it has access to the controls without passing them down to worker. Draft of solution:

private void prepareToRun()
{
    if (!backgroundWorker1.IsBusy)
    {
        backgroundWorker1 = new BackgroundWorker();
        backgroundWorker1.DoWork += backgroundWorker_DoWork;
        backgroundWorker1.ProgressChanged += backgroundWorker_ProgressChanged;
        backgroundWorker1.RunWorkerAsync();
    }
}

public static void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
{
    BackgroundWorker worker = sender as BackgroundWorker;

    // use worker.ReportProgress(percent, someData) somewhere here

}

private void backgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    // Update your statusStrip1 here according to data passed in e.ObjectState
}
\$\endgroup\$
  • \$\begingroup\$ This could be streamlined a little more by setting up your backgroundWorker1 control at Form1 Load, then just tell it RunWorkerAsync() if not IsBusy. \$\endgroup\$ – jp2code Jan 21 '14 at 19:15

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.