Countdown timer goes into negative number (00:00:00)

Question

I made a simple countdown timer but the timer goes into negative -1 : 59 : 59 when i input 0 : 0 : 0 on the textboxes. i tried to input 0 : 0 : 1 and the timer stopped at 0 : 0 : 0 and the messagebox appear on the screen

i have tried this code to prevent negative value but it stopped at -1 : 59 : 58

if (label1.Text == "-1")
{
    timer1.Stop()
}

tried this code but it stopped at -1 : 59 : 59

if (h < 0)
{
    timer1.Stop();
}

here is the codes

namespace Timer
{

    public partial class Form1 : Form
    {

        int h;
        int m;
        int s;
        public Form1()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            if (textBox1.Text == "")
            {
                textBox1.Text = "0";
            }
            if (textBox2.Text == "")
            {
                textBox2.Text = "0";
            }
            if (textBox3.Text == "")
            {
                textBox3.Text = "0";
            }

            h = Convert.ToInt32(textBox1.Text);
            m = Convert.ToInt32(textBox2.Text);
            s = Convert.ToInt32(textBox3.Text);

            timer1.Start();
        }

        private void timer1_Tick(object sender, EventArgs e)
        {
            s = s - 1;

            if(s == -1)
            {
                m = m - 1;
                s = 59;
            }

            if (m == -1)
            {
                h = h - 1;
                m = 59;
            }
            if (h == 0 && m == 0 && s == 0)
            {
                timer1.Stop();
                MessageBox.Show("Times up!", "Time");
            }

            string hh = Convert.ToString(h);
            string mm = Convert.ToString(m);
            string ss = Convert.ToString(s);

            label1.Text = hh;
            label2.Text = mm;
            label3.Text = ss;
        }

        private void button2_Click(object sender, EventArgs e)
        {
            timer1.Stop();
        }
    }
}

Answer 1

You code is doing exactly what you are telling it to do, the problem is that you are not dealing with the edge case of starting at 0:0:0.

I assume this would be considered an invalid input, so probably the easiest way to deal with this would be to check before you even start the timer in the button click:

        h = Convert.ToInt32(textBox1.Text);
        m = Convert.ToInt32(textBox2.Text);
        s = Convert.ToInt32(textBox3.Text);

        // one of these must be non-zero
        if (h != 0 || m != 0 || s != 0)
        {
            timer1.Start();
        }
        else
        {
            // handle this how ever you want but you don't need to start a timer
            // and really shouldn't start the timer
        }

It would actually be a mistake to let the timer tick if the user entered all zeros because then they would get 1 second, when they asked for 0 seconds.

Even better would be to actually disable the button until a non-zero time has been entered. To do that I would suggest replacing the TextBox with NumericUpDown (because only numeric inputs are valid anyway) and then add handlers for their ValueChanged event. In that handler check in any of the three controls have a non-zero value and enable the button if they do. If they are all zero, disable the button.

An important aside here - the System.Windows.Forms.Timer is not particularly accurate so don't expect a timer set to tick every 1 second to actually tick every one second. It will be at least 1 second between each tick, but often it will be a few ms more. So your countdown will drift. If you set it to count down 1 minute (i.e. 60 seconds), don't be surprised if it actually takes 62 seconds to count down. If that's important to you, then you should record the current time when you start the timer and then check the difference between the current time and the time when you started the timer and use that to update your labels.

A better overall solution might look something like this:

DateTime end;

private void button1_Click(object sender, EventArgs e)
{
    var h = hourNumericUpDown.Value;
    var m = minuteNumericUpDown.Value;
    var s = secondsNumericUpDown.Value;
    if (h != 0 || m != 0 || s != 0)
    {
        var start = DateTime.Now;
        var timeSpan = new TimeSpan(0,h,m,s);
        end = start.Add(timeSpan);
        countDownLabel.Text = timeSpan.ToString();  
        timer1.Start();
    }
}

private void timer1_Tick(object sender, EventArgs e)
{
    var timeleft = end - DateTime.Now;
    if (timeLeft.Ticks < 0) 
    {
        countDownLabel.Text = "00:00:00";
        timer1.Stop();
        MessageBox.Show("Times up!", "Time");
    }
    else 
    {
        countDownLabel.Text = string.Format("{0:D2}:{1:D2}:{2:D2}", 
            timeLeft.Hours, timeLeft.Minutes, timeLeft.Seconds);
    }
}

And then you'd probably be better off setting the timer to fire faster. Maybe every half second, or every quarter second so that the display is never off by more than about that much.

Answer 2

You're not covering the case in which 0:0:0 is provided. Replace this:

        s = s - 1;

        if(s == -1)
        {
            m = m - 1;
            s = 59;
        }

        if (m == -1)
        {
            h = h - 1;
            m = 59;
        }

with this:

if(s > 0 || m > 0 || h > 0)
{
            s = s - 1;
            if(s == -1)
            {
                m = m - 1;
                s = 59;
            }

            if (m == -1)
            {
                h = h - 1;
                m = 59;
            }
}

Answer 3

You are not checking when h becomes negative, I have added one you can add yours.

 if (s == -1)
        {
            m = m - 1;
            s = 59;
        }

        if (m == -1)
        {
            h = h - 1;
            m = 59;
        }
        /*I added such condition*/
        if(h < 0)
        {
            h = 0;
            m = 0;
            s = 0;
        }
        if (h == 0 && m == 0 && s == 0)
        {
            timer1.Stop();
            MessageBox.Show("Times up!", "Time");
            return;//return early
        }