2
\$\begingroup\$

I decided to make a stopwatch as I decided it would be useful, and also as a challenge to learn more about how forms work. I'd just like some simple advice as to anything I can do better, thank you.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace Stopwatch
{
    public partial class Form1 : Form
    {
        Timer timer = new Timer();

        int seconds = 0;
        int minutes = 0;
        int hours = 0;

        public Form1()
        {
            timer.Interval = 1000;
            timer.Tick += Timer_Tick;
            InitializeComponent();
        }

        private void Timer_Tick(object sender, EventArgs e)
        {
            IncrementStopwatch();
            UpdateLabels();
        }

        private void IncrementStopwatch()
        {
            seconds++;
            if (seconds == 60)
            {
                seconds = 0;
                minutes++;
                if (minutes == 60)
                {
                    minutes = 0;
                    hours++;
                }
            }
        }

        private void UpdateLabels()
        {
            Seconds.Text = KeepStyle(seconds);
            Minutes.Text = KeepStyle(minutes);
            Hours.Text = KeepStyle(hours);
        }

        private string KeepStyle(int s)
        {
            if (s <= 9)
                return "0" + s.ToString();
            return s.ToString();
        }

        private void StartStopButton_Click(object sender, EventArgs e)
        {
            timer.Enabled = !timer.Enabled;
            if (!timer.Enabled)
                StartStopButton.Text = "Start";
            else if (timer.Enabled)
                StartStopButton.Text = "Stop";
        }

        private void ResetButton_Click(object sender, EventArgs e)
        {
            seconds = 0;
            minutes = 0;
            hours = 0;
            UpdateLabels();
            timer.Enabled = !timer.Enabled; // Reset timer so it ticks in 1 second.
            timer.Enabled = !timer.Enabled;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Could you add a screenshot? \$\endgroup\$ – t3chb0t Sep 9 '18 at 9:37
4
\$\begingroup\$
  • Use System.Diagnostics.Stopwatch instead of keeping separate variables for hour, minute and seconds. It already has all the logic for Start/Stop/Reset, the logic of seconds overflowing to minutes. etc. Another benefit is that it is more accurate. However you will still need to have a timer to refresh the user interface.

  • Use consistent naming for form controls; buttons could end with "Button" and Labels could end with "Label".

  • Drag the Timer component from the toolbox to your form instead, this allow Windows.Forms to dispose it properly for you.

  • Remove unused usings.

    using System;
    using System.Windows.Forms;
    
    namespace Stopwatch
    {
        public partial class Form1 : Form
        {
            private System.Diagnostics.Stopwatch stopwatch = new System.Diagnostics.Stopwatch();
    
            public Form1()
            {
                InitializeComponent();
    
                // TODO: Move to winforms designer.
                timer.Tick += Timer_Tick;
                timer.Enabled = true;
            }
    
            private void Timer_Tick(object sender, EventArgs e)
            {
                var elapsed = stopwatch.Elapsed;
    
                SecondsLabel.Text = elapsed.Seconds.ToString("00");
                MinutesLabel.Text = elapsed.Minutes.ToString("00");
                HoursLabel.Text = elapsed.Hours.ToString("00");
            }
    
            private void StartStopButton_Click(object sender, EventArgs e)
            {
                if (stopwatch.IsRunning)
                {
                    StartStopButton.Text = "Start";
                    stopwatch.Stop();
                }
                else
                {
                    StartStopButton.Text = "Stop";
                    stopwatch.Start();
                }
            }
    
            private void ResetButton_Click(object sender, EventArgs e)
            {
                var isRunning  = stopwatch.IsRunning;
                stopwatch.Reset();
                if (isRunning)
                {
                    stopwatch.Start();
                }
            }
        }
    }
    
\$\endgroup\$
3
\$\begingroup\$

Instead of:

    private string KeepStyle(int s)
    {
        if (s <= 9)
            return "0" + s.ToString();
        return s.ToString();
    }

you can do:

s.ToString("00");

See here


Instead of:

    private void StartStopButton_Click(object sender, EventArgs e)
    {
        timer.Enabled = !timer.Enabled;
        if (!timer.Enabled)
            StartStopButton.Text = "Start";
        else if (timer.Enabled)
            StartStopButton.Text = "Stop";
    }

You can do:

    private void StartStopButton_Click(object sender, EventArgs e)
    {
        start = DateTime.Now; // See below
        timer.Enabled = !timer.Enabled;
        StartStopButton.Text = timer.Enabled ? "Stop" : "Start";
    }

Consider to use a DateTime object instead of your hours, minutes, seconds fields:

private DateTime start;

You can then skip the IncrementStopwatch() method and in UpdateLabels() do something like this:

    private void UpdateLabels()
    {
        TimeSpan duration = DateTime.Now - start;
        Seconds.Text = duration.Seconds.ToString("00");
        Minutes.Text = duration.Minutes.ToString("00");
        Hours.Text = duration.Hours.ToString("00");
    }

Remember to dispose the Timer object, when disposing the Form

protected override void Dispose(bool disposing)
    {
      if (disposing && (components != null))
      {
        components.Dispose();
      }
      if (disposing)
      {
        if (timer != null) timer.Dispose();
        timer = null;
      }
      base.Dispose(disposing);
    }

(Dispose(bool disposing) can be found in Form1.Designer.cs)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.