Stopwatch class without using System.Diagnostics in C#

Question

Design a class called Stopwatch. The job of this class is to simulate a stopwatch. It should provide two methods: Start and Stop. We call the start method first, and the stop method next. Then we ask the stopwatch about the duration between start and stop. Duration should be a value in TimeSpan. Display the duration on the console. We should also be able to use a stopwatch multiple times. So we may start and stop it and then start and stop it again. Make sure the duration value each time is calculated properly. We should not be able to start a stopwatch twice in a row (because that may overwrite the initial start time). So the class should throw an InvalidOperationException if its started twice.

The aim of this exercise is to make you understand that a class should be always in a valid state. We use encapsulation and information hiding to achieve that. The class should not reveal its implementation detail. It only reveals a little bit, like a blackbox. From the outside, you should not be able to misuse a class because you shouldn’t be able to see the implementation detail.

That is the information given to me from the tutorial I am taking to teach myself C#. I have completed this and looking for ways to improve my code and/or learn something new here. I have tested this and to my knowledge is working as expected. Any help is appreciated.

using System;

namespace ExerciseOne
{
    public static class Stopwatch
    {
        private static DateTime TimeStart { get; set; }
        private static DateTime TimeStop { get; set; }
        private static bool isStarted = false;
        private static bool isStopped = false;

        private static void StartTimer(DateTime start)
        {
            if (isStarted)
            {
                throw new InvalidOperationException("Unable to start a stopwatch twice in a row.");
            }
            else
            {
                isStarted = true;
                isStopped = false;
                TimeStart = start;
            }
        }

        private static void StopTimer(DateTime stop)
        {
            if (isStopped)
            {
                throw new InvalidOperationException("Unable to stop a stopwatch twice in a row.");
            }
            else
            {
                isStarted = false;
                isStopped = true;
                TimeStop = stop;
            }
        }

        private static string ElapsedTimer() => (TimeStop - TimeStart).ToString();

        private static void Begin()
        {
            StartTimer(DateTime.Now);
            System.Console.WriteLine(" - Stopwatch has begun.");
        }
        private static void End()
        {
            StopTimer(DateTime.Now);
            Console.WriteLine($" - Stopwatch has stopped. Elapsed Time: {ElapsedTimer()}");
        }

        public static void RunProgram()
        {
            Console.WriteLine("Stopwatch program.");
            Console.WriteLine("Type \"S\" to start the program. Type \"T\" to stop the program. Type \"E\" to end the program.");

            while (true)
            {
                ConsoleKeyInfo cki = Console.ReadKey(false);

                if (cki.Key == ConsoleKey.E)
                {
                    Console.WriteLine(": \"E\" key was pressed.  Progam exited.");
                    return;
                }
                else if (cki.Key == ConsoleKey.S)
                {
                    Begin();
                }
                else if (cki.Key == ConsoleKey.T)
                {
                    End();
                }
                else
                {
                    Console.WriteLine("\nPlease type either \"S\" to start the program, \"T\" to stop the program, \"E\" to end the program.");
                }
            }
        }
    }
}

using System;

namespace ExerciseOne
{
    class Program
    {
        static void Main() => Stopwatch.RunProgram();
    }
}

Answer 1

public static void RunProgram()
{
   ....
}

Why do you run the application as a method on the Stopwatch class it self?

---

You have made all the methods (StartTimer(), StopTimer()) private. This means that they only can be run from your RunProgram method, which occupy the main thread of the program and measures nothing. This set up makes the entire effort rather useless. I would expect a stopwatch being able to measure something in a way like:

  Stopwatch stopwatch = Stopwatch.StartNew();

  // TODO: execute something you want to measure
  Thread.Sleep(5000);

  stopwatch.Stop();

  Console.WriteLine($"Duration: {stopwatch.Duration}");

---

    private static bool isStarted = false;
    private static bool isStopped = false;

These flags can be combined into one isRunning, used like:

    private static void StartTimer(DateTime start)
    {
        if (isRunning)
        {
            throw new InvalidOperationException("Unable to start a stopwatch twice in a row.");
        }
        else
        {
            isRunning = true;
            TimeStart = start;
        }
    }

and

    private static void StopTimer(DateTime stop)
    {
        if (!isRunning)
        {
            throw new InvalidOperationException("Unable to stop the stopwatch because it is not running.");
        }
        else
        {
            isRunning = false;
            TimeStop = stop;
        }
    }

---

    private static void Begin()
    {
        StartTimer(DateTime.Now);
        System.Console.WriteLine(" - Stopwatch has begun.");
    }

You shouldn't write to the console in class methods of any object, unless it's explicitly meant to be a console application. The above message is useless in a WinForm application.

---

private static void StartTimer(DateTime start)

I don't se why you have a start time as argument to StartTimer. Why not just call DateTime.Now inside it? And likewise in StopTimer.

---

You were told to provide the result as a TimeSpan, but you actually return a string:

private static string ElapsedTimer() => (TimeStop - TimeStart).ToString();

---

public static class Stopwatch

The major problem with your implementation is though, that you make it static. That means that you only can run one "instance" at a time. In this way you aren't able to measure on two treads at the same time - or have nested measurements.

I would remove all the static stuff and only have one static method - starting a new Stopwatch instance, that then can be stopped by calling Stop() on the returned object like I showed above. So the public interface of the object would be:

  public class Stopwatch
  {
    public TimeSpan Duration { get; }

    public void Start() { }
    public void Stop() { }
    public static Stopwatch StartNew() { }
  }

I think you have misunderstood the concepts of "encapsulation and information hiding" slightly. An object must have a public interface through which clients can communicate with it, but they shouldn't be allowed to manipulate the objects internal state (for instance the StartTime member of your watch) - only through the public interface. The client shouldn't know about how you measure the time internally, they are only interested in the final result, when they stop the watch by calling Stop().

Answer 2

I agree with @Henrik_Hansen's answer. My answer builds upon his.

I absolutely don't see a reason for anything to be static.

Your Stopwatch class should only be for a stop watch. Right now you have commingled a UI into the stopwatch. In the name of Separation of Concerns, I see no reason for any Console.Write or Console.WriteLine in the Stopwatch class.

Going further, I see no reason for any method in Stopwatch to return a string. For Duration, simply return the TimeSpan value and leave it to whoever called that to decide what to do with it. Maybe they want it ToString(), but that's up to them. Maybe they want to add the Duration leg to cumulative TimeSpan.

Any internal time keeping calling DateTime should always use DateTime.UtcNow and not DateTime.Now. Granted UtcNow is faster than Now but in real-time that is laughably negligible. The main reason to do it is to avoid any Daylight Saving Time transitions where you start the stopwatch during standard time and stop it during DST. This would produce incorrect Duration.

A stopwatch really has only 1 state. It is either running or it is stopped. You do not need 2 variables for start and stop state. This runs the risk of you accidentally having conflicting states, e.g. the isStarted and isStopped are both true or both false. You only need 1 such variable. I personally prefer this:

public bool IsRunning { get; private set; }

Adding to Henrik's skeletal class, I would update it to:

public class Stopwatch
  {
    public TimeSpan Duration { get; }
    public bool IsRunning { get; private set; }

    public void Start() { }
    public void Stop() { }
    public static Stopwatch StartNew() { }
  }

The interesting thing you can do now with Duration is control the behavior of what happens if someone asks for the Duration while the stopwatch is running. Would you want to throw an exception? I personally would want to see the current Duration without stopping the stopwatch. This would produce a leg time while letting the stopwatch continue running.

Once you get this going, I would suggest you could add other nice methods. This would mirror Diagnostics.Stopwatch. Two such methods would be Reset and Restart.

Answer 3

This is my refactor based on all the suggestions made here. Works quite nice. Thank you all for your help and teaching me a few things.

Stopwatch.cs

using System;

namespace ExerciseOne
{
    public class Stopwatch
    {
        private DateTime timeStart;
        private DateTime timeStop;
        private bool isRunning;

        private void StartTimer()
        {
            if (isRunning)
            {
                throw new InvalidOperationException("Unable to start a stopwatch twice in a row.");
            }
            else
            {
                isRunning = true;
                timeStart = DateTime.UtcNow;
                timeStop = DateTime.UtcNow;
            }
        }

        private void StopTimer()
        {
            if (!isRunning)
            {
                throw new InvalidOperationException("Unable to stop a stopwatch twice in a row.");
            }
            else
            {
                isRunning = false;
                timeStop = DateTime.UtcNow;
            }
        }

        public TimeSpan ElapsedTimer => (timeStop - timeStart);
        public void Begin() => StartTimer();
        public void End() => StopTimer();
    }
}

Program.cs

using System;

namespace ExerciseOne
{
    class Program
    {
        static void Main()
        {
            Console.WriteLine("Stopwatch program.");
            Console.WriteLine("Type \"S\" to start the program. Type \"T\" to stop the program. Type \"E\" to end the program.");
            Stopwatch sw = new Stopwatch();

            while (true)
            {
                ConsoleKeyInfo cki = Console.ReadKey();

                if (cki.Key == ConsoleKey.E)
                {
                    Console.WriteLine(": \"E\" key was pressed.  Progam exited.");
                    return;
                }
                else if (cki.Key == ConsoleKey.T)
                {
                    sw.End();
                    Console.WriteLine($" - Stopwatch has stopped. Elapsed Time: {sw.ElapsedTimer}");
                }
                else if (cki.Key == ConsoleKey.S)
                {
                    sw.Begin();
                    System.Console.WriteLine(" - Stopwatch has begun.");
                }
                else
                {
                    Console.WriteLine("\nPlease type either \"S\" to start the program, \"T\" to stop the program, \"E\" to end the program.");
                }
            }
        }
    }
}