6

I have the follow button click event:

private void btnRun_Click(object sender, EventArgs e)
    {
        label1.Visible = true;

        if (SelectDatabase())
        {
            if (string.IsNullOrEmpty(txtFolderAddress.Text))
                MessageBox.Show("Please select a folder to begin the search.");
            else
            {

                if (cbRecurse.Checked == false || Directory.GetDirectories(initialDirectory).Length == 0)
                {
                    CheckSingleFolder();
                }
                else
                {
                    CheckSingleFolder();
                    directoryRecurse(initialDirectory);
                }

                                }
        }


    }

Effectively, it does a few checks and then starts some directory recursion looking for specific files. However, the very first line of code to make the label visible does not occur until after the directories have been recursed? Anybody know why this would be?

Thanks.

Darren Young
  • 10,972
  • 36
  • 91
  • 150

5 Answers5

8

You're doing everything within the UI thread, which is a really bad idea - the UI doesn't get to update, react to events etc until you've finished.

You should use a background thread and update the UI with progress etc using Control.BeginInvoke, or perhaps use a BackgroundWorker.

Basically, there are two golden rules in WinForms (and similar with WPF/Silverlight):

  • Don't do anything which can take a significant amount of time in the UI thread
  • Don't touch any UI elements from any thread other than the UI thread
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • What would be the best way then to wait for the new thread to finish? I have another method that has to wait until the recursion has finished. If I use a seperate thread and then use something like while(thread.isalive){}, it obviously blocks the thread until the new thread is finished. What would you recommend? Thanks. – Darren Young Apr 04 '11 at 13:49
  • @Darren: Make the file-handling code marshal a call back to the UI thread, calling whatever method needs to continue afterwards (e.g. re-enabling the button etc). – Jon Skeet Apr 04 '11 at 13:51
  • Excellent thanks. So I simply, forexample, use something as such: btnRun.Invoke((MethodInvoker)delegate { //Run Method }); – Darren Young Apr 04 '11 at 13:58
  • @Darren: Yes, that sort of thing. Personally I prefer using `BeginInvoke` unless you *really* need the worker thread to block, but that's a minor point :) – Jon Skeet Apr 04 '11 at 14:17
1

your whole method runs as a blocking unit currently - add an Application.DoEvents() as a workaround, but really you should be doing this kind of processing in a background thread, i.e. using a background worker.

BrokenGlass
  • 158,293
  • 28
  • 286
  • 335
  • 3
    Blech. Application.DoEvents is a hack - this code should not be executing on the UI thread to start with. – Jon Skeet Apr 04 '11 at 13:41
  • And a very *dangerous* hack, at that. I'm glad you added that clarification; you narrowly escaped a downvote from me. ;-) There are just too many answers making ill-conceived recommendations to use `Application.DoEvents`. – Cody Gray - on strike Apr 04 '11 at 13:47
1

The code is executing on the same thread which is drawing your user-interface. Therefore, while the code is executing, your UI is not being re-drawn. Once the button-click code has finished, the UI is redrawn and label1 gets drawn invisibly.

You can move your code into a separate thread using, for example, Task or BackgroundWorker. However, you cannot directly set UI properties from a different thread, so you will need to either be careful to set your UI properties from the UI thread or see this question about how to update the GUI from another thread.

Community
  • 1
  • 1
RB.
  • 36,301
  • 12
  • 91
  • 131
1

The view is not updated until the code block finished. So I would propose a BackgroundWorker for the recursion part.

1

The explanation: the label is set to visible and it it is Invalidated (needs repainting) but the Windows message pump doesn't start repainting until it is running idle. So your code blocks it.

A simple solution is to call label1.Update() right after setting it visible.

A better solution is to move the time-consuming code to a thread (Backgroundworker).

H H
  • 263,252
  • 30
  • 330
  • 514