How do I make a repeating button with a thread in c#?

Question

I've looked at some guides and none of them have gotten me all the way there. I've never made a thread, discussed a thread, or seen a thread at the grocery store, so this may be a problem. Currently. I'm trying:

private void btnHUp_MouseDown(object sender, MouseEventArgs e)
    {
        {
            ThreadStart HUp = new ThreadStart(dothis);
            t = new Thread(HUp);
            t.Start();
        }
    }
         public void dothis()
{
        if (intHour < 23)
        intHour = intHour += intStep;            
        lblTimerHour.Text = intHour.ToString("00");
        }
         private void btnHUp_MouseUp(object sender, MouseEventArgs e)
         {

             t.Abort();
         }
    }

That gets me InvalidOperationException was unhandled on the

 lblTimerHour.Text = intHour.ToString("00"); 

line. I read what that means and... it might as well be in Mandarin, I kind of get the general concept-ish of what's going wrong, but it's painfully fuzzy. If you asked me the first step in fixing it I'd look at you like a deer in the headlights. We just haven't gotten that far in my class yet.

Answer 1

The problem here is that the label you are trying to update is owned by the main thread (i.e. what the UI runs on), and that means that only that thread can access/update it. So, since you are in a different thread, you need to tell the UI thread to update the label for you.

Something like this would work:

Action updateLabel = () => lblTimerHour.Text = intHour.ToString("00");
lblTimerHour.BeginInvoke(updateLabel);

What this does is tell the lblTimerHour to invoke the action you define above (updateLabel).

Answer 2

See this post: How to update the GUI from another thread in C#?

lblTimerHour.Invoke((MethodInvoker)delegate {
    //Do what you need to do with the label
    lblTimerHour.Text = intHour.ToString("00");
});

Edit

This should do the trick:

public void dothis()
{
    do
    {
        if (intHour < 23)
            intHour = intHour += intStep;     

        lblTimerHour.Invoke((MethodInvoker)delegate {   
            //Update the label from the GUI thread    
            lblTimerHour.Text = intHour.ToString("00");
        });

        //Pause 1 sec. Won't freeze the gui since it's in another thread
        System.Thread.Sleep(1000);

    }while(true); //Thread is killed on mouse up
}

Answer 3

Well, let's take a look and see what you already have.

First, I see you did this.

private void btnHUp_MouseDown(object sender, MouseEventArgs e)
{
  ThreadStart HUp = new ThreadStart(dothis);
  t = new Thread(HUp);
  t.Start();
}

While this certainly is not the freshest stuff around it will still work. If you wanted some fresher ingredients then you might go with this instead.

private void btnHUp_MouseDown(object sender, MouseEventArgs e)
{
  Task.Factory.StartNew(dothis);
}

Second, I see this.

public void dothis()
{
  if (intHour < 23) intHour = intHour += intStep;            
  lblTimerHour.Text = intHour.ToString("00");
}

The problem here is that you are attempting to update a UI control from a thread other than the main UI thread. You see UI controls have what is called thread affinity. They can only ever be accessed from the thread that created them. What you have will lead to all kinds of unpredictable problems up to and including tearing a whole in spacetime.

A better option would be to do this.

public void dothis()
{
  while (intHour < 23) 
  {
    intHour = intHour += intStep;       
    lblTimerHour.Invoke((Action)(
      () =>
      {
        lblTimerHour.Text = intHour.ToString("00");
      }));
  }     
}

I assumed that you were missing the loop so I added it. While I cannot say that I personally have a taste for this kind of thing it is much easier to swallow. The real problem here is that the worker thread really does not do a whole lot of useful work. And then to top it off we have to use an awkward marshaling operation to transfer the result back to the UI thread. It is not pretty, but it will work.

And finally that brings me to this.

private void btnHUp_MouseUp(object sender, MouseEventArgs e)
{
   t.Abort();
}

You are attempting to abort a thread which is highly inadvisable. The problem is that it yanks control from the thread at unpredictable times. That thread might be in the middle of a write to data structure which would corrupt it. This is actually a pretty bad problem because any data structure in the process of being manipulated from any one of the frames on the call stack could be in an inconsistent state. This includes code you did not write. That is why it is hard to say what you may or may not be corrupting by doing this.

What you need to consider instead is using the cooperative cancellation mechanisms. This includes the use of CancellationTokenSource and CancellationToken. Here is how it might look once we put everything together.

private CancellationTokenSource cts = null;

private void btnHUp_MouseDown(object sender, MouseEventArgs e)
{
  cts = new CancellationTokenSource();
  Task.Factory.StartNew(() => dothis(cts.Token));
}

private void btnHUp_MouseUp(object sender, MouseEventArgs e)
{
   cts.Cancel();
}

public void dothis(CancellationToken token)
{
  while (!token.IsCancellationRequested) 
  {
    intHour += intStep;       
    lblTimerHour.Invoke((Action)(
      () =>
      {
        lblTimerHour.Text = intHour.ToString("00");
      }));
    Thread.Sleep(1000);
  }     
}

What this does is signal that the worker thread should gracefully shutdown on its own. This gives the worker thread a chance to tidy things up before eventually terminating itself.

Answer 4

If you want to update the UI every X period of time then there are already existing tools for this; a Timer will do exactly what you want, and it will be much more efficient and easier to code than creating a new thread that just spends most of its time napping. Additionally, aborting threads is a very bad sign to see. Avoid it at all costs.

First create the timer and configure it in the constructor:

private System.Windows.Forms.Timer timer = new System.Windows.Forms.Timer();
private int hour = 0;
private int step = 0;
public Form1()
{
    InitializeComponent();

    timer.Tick += timer_Tick;
    timer.Interval = 1000;
}

Have the Tick event do whatever should be done whenever it ticks.

private void timer_Tick(object sender, EventArgs e)
{
    if (hour < 23)
    {
        hour += step;
        lblTimerHour.Text = hour.ToString("00");
    }
}

Then just start the timer when you want it to start ticking and stop the timer when you want it to stop:

private void btnHUp_MouseDown(object sender, MouseEventArgs e)
{
    timer.Start();
}

private void btnHUp_MouseUp(object sender, MouseEventArgs e)
{
    timer.Stop();
}

The timer will automatically ensure that the Tick event handler runs in the UI thread, and it won't block the UI thread (or any other thread) when its waiting for the next event to happen, it will just do nothing.