Why does my timer not stop when checkbox is unchecked?

Question

Here's my code

private void checkBox1_CheckedChanged(object sender, EventArgs e)
    {
        System.Timers.Timer myTimer = new System.Timers.Timer();
        myTimer.Elapsed += new ElapsedEventHandler(DisplayTimeEvent);
        myTimer.Interval = Convert.ToInt32(textBox5.Text);
        if (checkBox1.Checked)
        {
            myTimer.Start();
        }
        else
        {
            myTimer.Stop();
        }

    }

It should stop repeating the function when unchecked, but it doesn't. What's wrong with it?

I have one child. I ask them to start drawing. Then I have **another (`new`) child** and I ask them to stop drawing. Did I ever ask the _first_ child to stop? No, no I didn't. — mjwills, Jun 06 '20 at 08:15
Use the [System.Windows.Forms.Timer](https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.timer?view=netcore-3.1). — , Jun 06 '20 at 13:22
This code obviously creates a new timer every time the CheckBox state is changed. Also, based on the text `DisplayTimeEvent`, there are likely UI-thread issues too. See [my answer](https://stackoverflow.com/a/62232924/1563833) for recommended repairs. — Wyck, Jun 06 '20 at 14:16

score 0 · Answer 1 · answered Jun 06 '20 at 08:58

Everytime checkbox1 is changed, new Timer is created. When checkbox is ticked, created timer is active and will invoke DisplayTimeEvent forever. When checkbox is unticked, you stop another Timer, which was just created.

You need to create Timer only once (probably when form is created), or when checkbox1 is changed first time:

private System.Timers.Timer myTimer;

private void checkBox1_CheckedChanged(object sender, EventArgs e)
{
    if (myTimer == null) {
        myTimer = new System.Timers.Timer();
        myTimer.Elapsed += new ElapsedEventHandler(DisplayTimeEvent);
        myTimer.Interval = Convert.ToInt32(textBox5.Text);
    }
    if (checkBox1.Checked)
    {
        myTimer.Start();
    }
    else
    {
        myTimer.Stop();
    }

}

Wyck · Accepted Answer · 2020-06-07T13:38:34.717

I recommend that you stop using System.Timers.Timer and start using a System.Windows.Forms.Timer component.

Begin by removing your myTimer-related code (the entire body of checkBox1_CheckedChanged will need to be replaced with code from below.)
Add a Timer component to your form using the designer and name it myTimer. This will add a System.Windows.Forms.Timer field to your form called myTimer.
Using the designer, set the Tick event handler of myTimer to DisplayTimeEvent. (Or add a new handler and replace its code with the code of your DisplayTimeEvent function.)
Then change your checkBox1_CheckedChange function to look like this:

private void checkBox1_CheckedChanged(object sender, EventArgs e)
{
    if (int.TryParse(textBox5.Text, out int interval)) {
        this.myTimer.Interval = interval;
    }
    this.myTimer.Enabled = checkBox1.Checked;
    this.textBox5.Enabled = !checkBox1.Checked;
}

I also recommend adding the following handler to textBox5 to perform the bare minimum validation so you can't crash your app by entering an interval of 0 or the empty string, or some text that is not an integer.

private void textBox5_TextChanged(object sender, EventArgs e)
{
    this.checkBox1.Enabled = (int.TryParse(textBox5.Text, out int interval) && interval > 0);
}

The System.Windows.Forms.Timer's Tick handler will be called in the UI thread, meaning it will be safe to do things like update labels of your form in that handler. In contrast to that, the System.Timers.Timer will be called on a worker thread and will require that you take on some some thread-management responsibilities you likely don't want to incur, such as invoking your UI updates back to the main UI thread. See Why there are 5 Versions of Timer Classes in .NET? for more info.

Why does my timer not stop when checkbox is unchecked?

2 Answers2