C# - Winform Timer - Disposing and emptying the timer

Viewed 713

Fairly new to C# and timers, although I've managed to do some really fun stuff in C#, however I'm not getting the hang of Timers.

Form1.cs:

private int counter;
static System.Windows.Forms.Timer timer1 = new System.Windows.Forms.Timer();
public void goTimer()
{
    // Set Counter
    counter = 60;

    // If timer is already enabled, stop it.
    if (timer1.Enabled)
    {
        timer1.Dispose();
        //timer1.Stop() <- also tried

    }

    timer1.Tick += new EventHandler(timer1_Tick);
    timer1.Interval = 1000; // 1 second
    timer1.Start(); // Timer exists

    txtCountdown.Text = counter.ToString();

}

private void timer1_Tick(object sender, EventArgs e)
{
    counter--;
    if(counter == 0)
    {
        timer1.Stop();
    }
    txtCountdown.Text = counter.ToString();
}

So, what happens is that it seems to work as intended, until you start calling goTimer(); from e.g. a button press, then it will speed up the (int) counter as many times as you pressed it... And after a while the memory will be eaten up.

In this case the users will be able to do call the timer function, as it will remove some objects, clear some data and refresh the session, but also when the timer reaches 0.

Using Winforms, I did not add a timer in visual studio (it's only referenced here in Form1.cs).

How do I terminate all timers, and then restart at (int) counter?

2 Answers

Using start and stop of the timer would be the proper aproach, but generally also the dispose variant will work.

Your memory hole results from the multiplied event handler assignments, you need to move this method to your constructor or some other initialization method:

timer1.Tick += new EventHandler(timer1_Tick);

If you really want to create a new timer every time, you need to release the event handler before:

timer1.Tick -= timer1_Tick;

First of all, as MichaelSander already mentioned, you should put these lines in your Form1.cs constructor:

timer1.Tick += new EventHandler(timer1_Tick); timer1.Interval = 1000; // 1 second

Secondly, there is no point in disposing your timer if it's meant to be used more than once. Instead of timer1.Dispose() you should use timer1.Stop() just like you do in your timer1_Tick handler. Also there is no point in checking whether the timer is enabled or disabled as both timer1.Start() and timer1.Stop() will either turn it on/off respectively or do nothing at all. That means that in your case you can remove this block completely:

if (timer1.Enabled) { timer1.Dispose(); }

Related