Form timer doesn't start

Question

So I just started learning C# and using forms. I have been able to create a digital clock and tinker with this and that, but now I'm trying to make a basic UI for a derpy game and my timer doesn't work.

First - what I'm trying to accomplish: A simple decrementing timer from 60 seconds (*clock style (mm:ss)).
Second, here's what I have:

public partial class Form1 : Form
{
    private int counter = 60;
    public Form1()
    {
        InitializeComponent();
        label1.Text = TimeSpan.FromMinutes(1).ToString("m\\:ss");
    }

    private void pictureBox2_Click(object sender, EventArgs e)
    {

    }

    private void timer1_Tick(object sender, EventArgs e)
    {
        counter--;
        if (counter == 0)
        {
            timer1.Stop();
            label1.Text = counter.ToString();
            MessageBox.Show("Time's Up!!");
        }
    }

    private void label1_Click(object sender, EventArgs e)
    {
        var startTime = DateTime.Now;
        var counter = (TimeSpan.FromMinutes(1)).ToString("m\\:ss");
        timer1 = new Timer();
        timer1.Tick += new EventHandler(timer1_Tick);
        timer1.Interval = 1000;
        timer1.Start();
        label1.Text = counter.ToString();
    }
}

Appreciate the feedback and knowledge!

Answer 1

From the codes that I see, your timer is working but you are not updating it in each count, you are updating when the timer finishes -

private void timer1_Tick(object sender, EventArgs e)
{
    counter--;
    if (counter == 0)
    {
        timer1.Stop();
        label1.Text = counter.ToString(); // *** Look here
        MessageBox.Show("Time's Up!!");
    }
}

You should update the timer in each tick, so take the update label code out of the if block -

private void timer1_Tick(object sender, EventArgs e)
{
    counter--;
    label1.Text = counter.ToString(); // should work
    if (counter == 0)
    {
        timer1.Stop();
        MessageBox.Show("Time's Up!!");
    }
}

and also reset the counter in each cycle -

private void label1_Click(object sender, EventArgs e)
{
    var startTime = DateTime.Now;

    var counter = (TimeSpan.FromMinutes(1)).ToString("m\\:ss");
    timer1 = new Timer();
    timer1.Tick += new EventHandler(timer1_Tick);
    timer1.Interval = 1000;
    timer1.Start();
    label1.Text = counter.ToString();
    this.counter = 60
}

NOTE: I am really not sure if this code will throw any access violation error, due to updating the UI in a different thread or not. If so, then you have to use async/await or events/delegates to update UI.

Let me know, if this throws error, then I will give you the async/await version.

Answer 2

This works fine for me. I would give progress updates as the time is countdown to show that it is working. For example, if you did this in label, you could do something like the following:

private void timer1_Tick(object sender, EventArgs e)
{
    counter--;
    label1.Text = counter.ToString();
    if (counter == 0)
    {
        timer1.Stop();
        MessageBox.Show("Time's Up!!");
    }
}

Notice that the label1.Text = counter.ToString(); line has been moved before the counter == 0 check, so that it is able to provide feedback for all counter values.

As well, you may accidentally launch several timer1 instances if you do not keep track of how many you spawn using new Timer(). There are various ways to do this, but you could simply check whether timer1 already exists and counter == 0 before creating a new instance. You could perform this check as a guard clause (ie. return if either of those conditions are matched).

private void label1_Click(object sender, EventArgs e)
{
    var startTime = DateTime.Now;
    if (timer1 == null || (timer1 != null && counter == 0)) return;
    counter = (TimeSpan.FromMinutes(1)).ToString("m\\:ss");
    timer1 = new Timer();
    timer1.Tick += new EventHandler(timer1_Tick);
    timer1.Interval = 1000;
    timer1.Start();
    label1.Text = counter.ToString();
}

If you want this countdown to start automatically, you can put this directly into the constructor, or put it into another method and call it from the constructor like so:

public Form1()
{
    InitializeComponent();
    StartCountdown();
}

private void StartCountdown()
{
    var startTime = DateTime.Now;
    /* the rest of your original label1_Click code goes here ... */
}