views:

2823

answers:

13

I want to display a progress bar while doing some work, but that would hang the UI and the progress bar won't update.

I have a WinForm ProgressForm with a ProgressBar that will continue indefinitely in a marquee fashion.

using(ProgressForm p = new ProgressForm(this))
{
//Do Some Work
}

Now there are many ways to solve the issue, like using BeginInvoke, wait for the task to complete and call EndInvoke. Or using the BackgroundWorker or Threads.

I am having some issues with the EndInvoke, though that's not the question. The question is which is the best and the simplest way you use to handle such situations, where you have to show the user that the program is working and not unresponsive, and how do you handle that with simplest code possible that is efficient and won't leak, and can update the GUI.

Like BackgroundWorker needs to have multiple functions, declare member variables, etc. Also you need to then hold a reference to the ProgressBar Form and dispose of it.

Edit: BackgroundWorker is not the answer because it may be that I don't get the progress notification, which means there would be no call to ProgressChanged as the DoWork is a single call to an external function, but I need to keep call the Application.DoEvents(); for the progress bar to keep rotating.

The bounty is for the best code solution for this problem. I just need to call Application.DoEvents() so that the Marque progress bar will work, while the worker function works in the Main thread, and it doesn't return any progress notification. I never needed .NET magic code to report progress automatically, I just needed a better solution than :

Action<String, String> exec = DoSomethingLongAndNotReturnAnyNotification;
IAsyncResult result = exec.BeginInvoke(path, parameters, null, null);
while (!result.IsCompleted)
{
  Application.DoEvents();
}
exec.EndInvoke(result);

that keeps the progress bar alive (means not freezing but refreshes the marque)

A: 

Use the BackgroundWorker component it is designed for exactly this scenario.

You can hook into its progress update events and update your progress bar. The BackgroundWorker class ensures the callbacks are marshalled to the UI thread so you don't need to worry about any of that detail either.

Paolo
+12  A: 

For me the easiest way is definitely to use a BackgroundWorker, which is specifically designed for this kind of task. The ProgressChanged event is perfectly fitted to update a progress bar, without worrying about cross-thread calls

Thomas Levesque
but that needs lot of additional work in comparison to Begin and EndInvoke. See my update in the question.
Priyank Bolia
Well, you don't *have* to declare a member variable, and you can use lambda expressions for event handlers...
Thomas Levesque
a background worker is undoubtedly the easiest option
Pondidum
This is one of my favorite component!
Gabriel Mongeon
A: 

We are use modal form with BW for such a thing

here is quick solution

  public class ProgressWorker<TArgument> : BackgroundWorker where TArgument : class 
    {
        public Action<TArgument> Action { get; set; }

        protected override void OnDoWork(DoWorkEventArgs e)
        {
            if (Action!=null)
            {
                Action(e.Argument as TArgument);
            }
        }
    }


public sealed partial class ProgressDlg<TArgument> : Form where TArgument : class
{
    private readonly Action<TArgument> action;

    public Exception Error { get; set; }

    public ProgressDlg(Action<TArgument> action)
    {
        if (action == null) throw new ArgumentNullException("action");
        this.action = action;
        //InitializeComponent();
        //MaximumSize = Size;
        MaximizeBox = false;
        Closing += new System.ComponentModel.CancelEventHandler(ProgressDlg_Closing);
    }
    public string NotificationText
    {
        set
        {
            if (value!=null)
            {
                Invoke(new Action<string>(s => Text = value));  
            }

        }
    }
    void ProgressDlg_Closing(object sender, System.ComponentModel.CancelEventArgs e)
    {
        FormClosingEventArgs args = (FormClosingEventArgs)e;
        if (args.CloseReason == CloseReason.UserClosing)
        {
            e.Cancel = true;
        }
    }



    private void ProgressDlg_Load(object sender, EventArgs e)
    {

    }

    public void RunWorker(TArgument argument)
    {
        System.Windows.Forms.Application.DoEvents();
        using (var worker = new ProgressWorker<TArgument> {Action = action})
        {
            worker.RunWorkerAsync();
            worker.RunWorkerCompleted += worker_RunWorkerCompleted;                
            ShowDialog();
        }
    }

    void worker_RunWorkerCompleted(object sender, System.ComponentModel.RunWorkerCompletedEventArgs e)
    {
        if (e.Error != null)
        {
            Error = e.Error;
            DialogResult = DialogResult.Abort;
            return;
        }

        DialogResult = DialogResult.OK;
    }
}

and how we use it

var dlg = new ProgressDlg<string>(obj =>
                                  {
                                     //DoWork()
                                     Thread.Sleep(10000);
                                     MessageBox.Show("Background task completed "obj);
                                   });
dlg.RunWorker("SampleValue");
if (dlg.Error != null)
{
  MessageBox.Show(dlg.Error.Message, "ERROR", MessageBoxButtons.OK, MessageBoxIcon.Error);
}
dlg.Dispose();
Sergey Mirvoda
Do you have a sample application, its hard for me to understand this code, what is ProgressWorker?
Priyank Bolia
oh, sorry for that. ProgressWorker is a BackgroundWorker with Action as a constructor parameter. I'll modify sample.
Sergey Mirvoda
@Priyank Bolia code updated.
Sergey Mirvoda
+2  A: 

Indeed you are on the right track. You should use another thread, and you have identified the best ways to do that. The rest is just updating the progress bar. In case you don't want to use BackgroundWorker like others have suggested, there is one trick to keep in mind. The trick is that you cannot update the progress bar from the worker thread because UI can be only manipulated from the UI thread. So you use the Invoke method. It goes something like this (fix the syntax errors yourself, I'm just writing a quick example):

class MyForm: Form
{
    private void delegate UpdateDelegate(int Progress);

    private void UpdateProgress(int Progress)
    {
        if ( this.InvokeRequired )
            this.Invoke((UpdateDelegate)UpdateProgress, Progress);
        else
            this.MyProgressBar.Progress = Progress;
    }
}

The InvokeRequired property will return true on every thread except the one that owns the form. The Invoke method will call the method on the UI thread, and will block until it completes. If you don't want to block, you can call BeginInvoke instead.

Vilx-
A: 

Reading your requirements the simplest way would be to display a mode-less form and use a standard System.Windows.Forms timer to update the progress on the mode-less form. No threads, no possible memory leaks.

As this only uses the one UI thread, you would also need to call Application.DoEvents() at certain points during your main processing to guarantee the progress bar is updated visually.

Ash
+6  A: 

There's a load of information about threading with .NET/C# on Stackoverflow, but the article that cleared up windows forms threading for me was our resident oracle, Jon Skeet's "Threading in Windows Forms".

The whole series is worth reading to brush up on your knowledge or learn from scratch.

I'm impatient, just show me some code

As far as "show me the code" goes, below is how I would do it with C# 3.5. The form contains 4 controls:

  • a textbox
  • a progressbar
  • 2 buttons: "buttonLongTask" and "buttonAnother"

buttonAnother is there purely to demonstrate that the UI isn't blocked while the count-to-100 task is running.

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

    private void buttonLongTask_Click(object sender, EventArgs e)
    {
     Thread thread = new Thread(LongTask);
     thread.IsBackground = true;
     thread.Start();
    }

    private void buttonAnother_Click(object sender, EventArgs e)
    {
     textBox1.Text = "Have you seen this?";
    }

    private void LongTask()
    {
     for (int i = 0; i < 100; i++)
     {
      Update1(i);
      Thread.Sleep(500);
     }
    }

    public void Update1(int i)
    {
     if (InvokeRequired)
     {
      this.BeginInvoke(new Action<int>(Update1), new object[] { i });
      return;
     }

     progressBar1.Value = i;
    }
}
Chris S
To be honest, this is nothing different than the other answers, you just used Thread instead of BackgroundWorker, but I don't think this relates to my problem.
Priyank Bolia
+4  A: 

And another example that BackgroundWorker is the right way to do it...

using System;
using System.ComponentModel;
using System.Threading;
using System.Windows.Forms;

namespace SerialSample
{
    public partial class Form1 : Form
    {
        private BackgroundWorker _BackgroundWorker;
        private Random _Random;

        public Form1()
        {
            InitializeComponent();
            _ProgressBar.Style = ProgressBarStyle.Marquee;
            _ProgressBar.Visible = false;
            _Random = new Random();

            InitializeBackgroundWorker();
        }

        private void InitializeBackgroundWorker()
        {
            _BackgroundWorker = new BackgroundWorker();
            _BackgroundWorker.WorkerReportsProgress = true;

            _BackgroundWorker.DoWork += (sender, e) => ((MethodInvoker)e.Argument).Invoke();
            _BackgroundWorker.ProgressChanged += (sender, e) =>
                {
                    _ProgressBar.Style = ProgressBarStyle.Continuous;
                    _ProgressBar.Value = e.ProgressPercentage;
                };
            _BackgroundWorker.RunWorkerCompleted += (sender, e) =>
            {
                if (_ProgressBar.Style == ProgressBarStyle.Marquee)
                {
                    _ProgressBar.Visible = false;
                }
            };
        }

        private void buttonStart_Click(object sender, EventArgs e)
        {
            _BackgroundWorker.RunWorkerAsync(new MethodInvoker(() =>
                {
                    _ProgressBar.BeginInvoke(new MethodInvoker(() => _ProgressBar.Visible = true));
                    for (int i = 0; i < 1000; i++)
                    {
                        Thread.Sleep(10);
                        _BackgroundWorker.ReportProgress(i / 10);
                    }
                }));
        }
    }
}
Oliver
Not impressed, the question clearly states 'I don't get the progress notification' which means, there won't be any call to the ProgressChanged.
Priyank Bolia
@Priyank Bolia: You call the background worker's 'ReportProgress' function from your worker thread in order to invoke ProgressChanged. Also make sure 'WorkerReportsProgress' is true before the work begins, as Oliver shows in his example. http://msdn.microsoft.com/en-us/library/ka89zff4.aspx
YotaXP
@Priyank Bolia: Currently don't understand what you mean with *there won't be any call to the ProgressChanges*. Do you mean my example? If yes, just uncomment the .ReportProgress line. If you mean your code, enable `WorkerReportsProgress` and call `worker.ReportProgress()` within your `DoWork()`
Oliver
@Priyank Bolia: Updated sample to better report progress
Oliver
A: 

Re: Your edit. You need a BackgroundWorker or Thread to do the work, but it must call ReportProgress() periodically to tell the UI thread what it is doing. DotNet can't magically work out how much of the work you have done, so you have to tell it (a) what the maximum progress amount you will reach is, and then (b) about 100 or so times during the process, tell it which amount you are up to. (If you report progress fewer than 100 times, the progess bar will jump in large steps. If you report more than 100 times, you will just be wasting time trying to report a finer detail than the progress bar will helpfully display)

If your UI thread can happily continue while the background worker is running, then your work is done.

However, realistically, in most situations where the progress indication needs to be running, your UI needs to be very careful to avoid a re-entrant call. e.g. If you are running a progress display while exporting data, you don't want to allow the user to start exporting data again while the export is in progress.

You can handle this in two ways:

  • The export operation checks to see if the background worker is running, and disabled the export option while it is already importing. This will allow the user to do anything at all in your program except exporting - this could still be dangerous if the user could (for example) edit the data that is being exported.

  • Run the progress bar as a "modal" display so that your program reamins "alive" during the export, but the user can't actually do anything (other than cancel) until the export completes. DotNet is rubbish at supporting this, even though it's the most common approach. In this case, you need to put the UI thread into a busy wait loop where it calls Application.DoEvents() to keep message handling running (so the progress bar will work), but you need to add a MessageFilter that only allows your application to respond to "safe" events (e.g. it would allow Paint events so your application windows continue to redraw, but it would filter out mouse and keyboard messages so that the user can't actually do anything in the proigram while the export is in progress. There are also a couple of sneaky messages you'll need to pass through to allow the window to work as normal, and figuring these out will take a few minutes - I have a list of them at work, but don't have them to hand here I'm afraid. It's all the obvious ones like NCHITTEST plus a sneaky .net one (evilly in the WM_USER range) which is vital to get this working).

The last "gotcha" with the awful dotNet progress bar is that when you finish your operation and close the progress bar you'll find that it usually exits when reporting a value like "80%". Even if you force it to 100% and then wait for about half a second, it still may not reach 100%. Arrrgh! The solution is to set the progress to 100%, then to 99%, and then back to 100% - when the progress bar is told to move forwards, it animates slowly towards the target value. But if you tell it to go "backwards", it jumps immediately to that position. So by reversing it momentarily at the end, you can get it to actually show the value you asked it to show.

Jason Williams
I just need to call Application.DoEvents() so that the Marque progress bar will work, while the worker function works in the Main thread, and it doesn't return any progress notification. I never needed .NET magic code to report progress automatically, I just needed a better solution than BeginInvoke and EndInvoke that keeps the progress bar alive (means not freezing but refreshes the marque)
Priyank Bolia
A: 

I have to throw the simplest answer out there. You could always just implement the progress bar and have no relationship to anything of actual progress. Just start filling the bar say 1% a second, or 10% a second whatever seems similar to your action and if it fills over to start again.

This will atleast give the user the appearance of processing and make them understand to wait instead of just clicking a button and seeing nothing happen then clicking it more.

Chris Marisic
This is one of the most hateful user experiences I know of: a progress bar that lies to you about how long you have to wait.
Robert Rossney
@Robert from the comments of the poster it sounds like has little to no idea of the status of the operation in said question that this sounds like it might be the most useful answer anyway. But yes it is frustrating to watch a bar, waiting for it to fill over and see it recycle. Of course you could cycle through colors or something to atleast make it have some difference to look like it actually get to stage 2 etc.
Chris Marisic
The user experience is very frequently unrelated to actual operation of the program and entirely just subjective opinions ABOUT the program.
Chris Marisic
A: 

If you do end up using BackgroundWorker, consider doing it this way:

Instead of using BackgroundWorker as-is, consider inheriting from it...

Inheriting from BackgroundWorker makes it easy to pass in any information you need in your inheriting class's constructor, and you can hold state in your inheriting class's private member variables. You can also create a property or method to make it easy to get your result data back out.

Kyralessa
+1  A: 

BackgroundWorker is not the answer because it may be that I don't get the progress notification...

What on earth does the fact that you're not getting progress notification have to do with the use of BackgroundWorker? If your long-running task doesn't have a reliable mechanism for reporting its progress, there's no way to reliably report its progress.

The simplest possible way to report progress of a long-running method is to run the method on the UI thread and have it report progress by updating the progress bar and then calling Application.DoEvents(). This will, technically, work. But the UI will be unresponsive between calls to Application.DoEvents(). This is the quick and dirty solution, and as Steve McConnell observes, the problem with quick and dirty solutions is that the bitterness of the dirty remains long after the sweetness of the quick is forgotten.

The next simplest way, as alluded to by another poster, is to implement a modal form that uses a BackgroundWorker to execute the long-running method. This provides a generally better user experience, and it frees you from having to solve the potentially complicated problem of what parts of your UI to leave functional while the long-running task is executing - while the modal form is open, none of the rest of your UI will respond to user actions. This is the quick and clean solution.

But it's still pretty user-hostile. It still locks up the UI while the long-running task is executing; it just does it in a pretty way. To make a user-friendly solution, you need to execute the task on another thread. The easiest way to do that is with a BackgroundWorker.

This approach opens the door to a lot of problems. It won't "leak," whatever that is supposed to mean. But whatever the long-running method is doing, it now has to do it in complete isolation from the pieces of the UI that remain enabled while it's running. And by complete, I mean complete. If the user can click anywhere with a mouse and cause some update to be made to some object that your long-running method ever looks at, you'll have problems. Any object that your long-running method uses which can raise an event is a potential road to misery.

It's that, and not getting BackgroundWorker to work properly, that's going to be the source of all of the pain.

Robert Rossney
Such long note, but nothing of use to me, when did I say 'reliable mechanism for reporting its progress' I just need to Application.DoEvents() so that the Marque progress bar will work. Also not all long operation can be called from another thread, like I get lot of issue while calling SAPI on another thread.
Priyank Bolia
Your question is vague. You're not going to get useful answers until you're more specific.
Robert Rossney
+3  A: 

It seems to me that you are operating on at least one false assumption.

1. You don't need to raise the ProgressChanged event to have a responsive UI

In your question you say this:

BackgroundWorker is not the answer because it may be that I don't get the progress notification, which means there would be no call to ProgressChanged as the DoWork is a single call to an external function . . .

Actually, it does not matter whether you call the ProgressChanged event or not. The whole purpose of that event is to temporarily transfer control back to the GUI thread to make an update that somehow reflects the progress of the work being done by the BackgroundWorker. If you are simply displaying a marquee progress bar, it would actually be pointless to raise the ProgressChanged event at all. The progress bar will continue rotating as long as it is displayed because the BackgroundWorker is doing its work on a separate thread from the GUI.

(On a side note, DoWork is an event, which means that it is not just "a single call to an external function"; you can add as many handlers as you like; and each of those handlers can contain as many function calls as it likes.)

2. You don't need to call Application.DoEvents to have a responsive UI

To me it sounds like you believe that the only way for the GUI to update is by calling Application.DoEvents:

I need to keep call the Application.DoEvents(); for the progress bar to keep rotating.

This is not true in a multithreaded scenario; if you use a BackgroundWorker, the GUI will continue to be responsive (on its own thread) while the BackgroundWorker does whatever has been attached to its DoWork event. Below is a simple example of how this might work for you.

private void ShowProgressFormWhileBackgroundWorkerRuns() {
    // this is your presumably long-running method
    Action<string, string> exec = DoSomethingLongAndNotReturnAnyNotification;

    ProgressForm p = new ProgressForm(this);

    BackgroundWorker b = new BackgroundWorker();

    // set the worker to call your long-running method
    b.DoWork += (object sender, DoWorkEventArgs e) => {
        exec.Invoke(path, parameters);
    };

    // set the worker to close your progress form when it's completed
    b.RunWorkerCompleted += (object sender, RunWorkerCompletedEventArgs e) => {
        if (p != null && p.Visible) p.Close();
    };

    // now actually show the form
    p.Show();

    // this only tells your BackgroundWorker to START working;
    // the current (i.e., GUI) thread will immediately continue,
    // which means your progress bar will update, the window
    // will continue firing button click events and all that
    // good stuff
    b.RunWorkerAsync();
}

3. You can't run two methods at the same time on the same thread

You say this:

I just need to call Application.DoEvents() so that the Marque progress bar will work, while the worker function works in the Main thread . . .

What you're asking for is simply not real. The "main" thread for a Windows Forms application is the GUI thread, which, if it's busy with your long-running method, is not providing visual updates. If you believe otherwise, I suspect you misunderstand what BeginInvoke does: it launches a delegate on a separate thread. In fact, the example code you have included in your question to call Application.DoEvents between exec.BeginInvoke and exec.EndInvoke is redundant; you are actually calling Application.DoEvents repeatedly from the GUI thread, which would be updating anyway. (If you found otherwise, I suspect it's because you called exec.EndInvoke right away, which blocked the current thread until the method finished.)

So yes, the answer you're looking for is to use a BackgroundWorker.

You could use BeginInvoke, but instead of calling EndInvoke from the GUI thread (which will block it if the method isn't finished), pass an AsyncCallback parameter to your BeginInvoke call (instead of just passing null), and close the progress form in your callback. Be aware, however, that if you do that, you're going to have to invoke the method that closes the progress form from the GUI thread, since otherwise you'll be trying to close a form, which is a GUI function, from a non-GUI thread. But really, all the pitfalls of using BeginInvoke/EndInvoke have already been dealt with for you with the BackgroundWorker class, even if you think it's ".NET magic code" (to me, it's just an intuitive and useful tool).

Dan Tao
This is the only explanation that makes sense to me. I'll just wait for some time, before closing. Many thanks.
Priyank Bolia
A: 

If you want a "rotating" progress bar, why not set the progress bar style to "Marquee" and using a BackgroundWorker to keep the UI responsive? You won't achieve a rotating progress bar easier than using the "Marquee" - style...

Thorsten Dittmar