HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

BackgroundWorker vs TPL ProgressBar Exercise

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
backgroundworkerexercisetplprogressbar

Problem

I wanted to fiddle around with the BackgroundWorker and Task classes, to see how I would implement a given task with these two techniques.

So I created a new WinForms project, and implemented a simple UI; two sections, each with a ProgressBar, and Start + Cancel buttons:

I implemented a DoSomething-type "service":

public class SomeService
{
    public void SomeMethod()
    {
        Thread.Sleep(1000);
    }
}


But that's irrelevant. The form's code-behind is where I put all the code this post is all about:
Constructor

The form's constructor is essentially the entry point here (program.cs is ignored), so I put the obvious fields in first, and initialized them in the constructor:

public partial class Form1 : Form
{
    private readonly SomeService _service;
    private readonly BackgroundWorker _worker;

    public Form1()
    {
        _service = new SomeService();
        _worker = new BackgroundWorker { WorkerReportsProgress = true, WorkerSupportsCancellation = true };

        _worker.DoWork += OnBackgroundDoWork;
        _worker.ProgressChanged += OnBackgroundProgressChanged;
        _worker.RunWorkerCompleted += OnBackgroundWorkerCompleted;

        InitializeComponent();
        
        CloseButton.Click += CloseButton_Click;
        
        StartBackgroundWorkerButton.Click += StartBackgroundWorkerButton_Click;
        CancelBackgroundWorkerButton.Click += CancelBackgroundWorkerButton_Click;

        StartTaskButton.Click += StartTaskButton_Click;
        CancelTaskButton.Click += CancelTaskButton_Click;
    }

    private void CloseButton_Click(object sender, EventArgs e)
    {
        CancelBackgroundWorkerButton_Click(null, EventArgs.Empty);
        CancelTaskButton_Click(null, EventArgs.Empty);
        Close();
    }


#region BackgroundWorker

Button.Click handlers

These are the event handlers for the Start and Cancel buttons' Click event:

```
private void StartBackgroundWorkerButton_Click(object sender, EventArgs e)
{

Solution

It does show very good example for the progressbar in WinForms.
Here are my code-review outputs for you.

  • Closebutton_Click calls the other click "handlers". This is not the common way. Implement another method and call it from those handlers.



  • I couldnt understand the logic why are you asking the cancellation before and after invoking the DoSomething().



  • Reduce synchronization. Try to "synchron" your threads at once. Optimize method invocation delegate for progressbar.value and the canceltaskbutton.enabled.



  • Your .ContinueWith approach is good. But always eliminate 'switch’s from your code. Here would be a dictionary very handy. You can define your status-action set before hand and .ContinueWith status keyed action. This would reduce the complexity and increase the readability of your code.



-
On the OnBackgroundWorkerCompleted method, the Else-if should be separated from the first if-expression. (Again complexity and readability) Please see following code.

if (e.Error != null)
{
    // Handle Error and call reset.
    return; 
}

if (e.Cancelled)
{
    // Handle cancel and call reset.
    return;
}

if (e.Result == null)
{
    // No Result
}
else
{
    // Result
}

// reset ...

Code Snippets

if (e.Error != null)
{
    // Handle Error and call reset.
    return; 
}

if (e.Cancelled)
{
    // Handle cancel and call reset.
    return;
}


if (e.Result == null)
{
    // No Result
}
else
{
    // Result
}

// reset ...

Context

StackExchange Code Review Q#41295, answer score: 6

Revisions (0)

No revisions yet.