Simple stopwatch class

Question

I'm not sure what the best way is when dealing with users calling start() multiple times or stop() before calling start(), I decided to silently return. I lean towards not using exceptions for these situations. Is there a good argument to be made for exceptions? Is there a different or better mechanism?

Looking for:

• Suggestions on missing operations (Provide bool has_started() noexcept;?).
• Handling special cases (calling start on a stopwatch that has already started, etc.)
• General suggestions: Better idioms, code clarity, etc.

---

#ifndef CR_STOPWATCH_H
#define CR_STOPWATCH_H

#include <chrono>
//#include <stdexcept>

namespace cr
{
    /*
        DECLARATION
    */

    template
    <
        typename TimeUnit = std::chrono::milliseconds,
        typename Clock = std::chrono::high_resolution_clock
    >
    class stopwatch
    {
    public:
        explicit stopwatch( bool const = false ) noexcept;

        void start() noexcept;
        void stop() noexcept;

        TimeUnit elapsed() noexcept;
        void reset() noexcept;

        template <typename F, typename... FArgs>
        inline static TimeUnit measure( F&&, FArgs&&... );

    private:
        bool stopped_;
        std::chrono::time_point<Clock> stop_;

        bool started_;
        std::chrono::time_point<Clock> start_;
    };

    /*
        IMPLEMENTATION
    */

    template <typename TimeUnit, typename Clock>
    inline cr::stopwatch<TimeUnit, Clock>::stopwatch( bool const start_stopwatch = false ) noexcept :
        stopped_{ false },
        stop_{ TimeUnit{ 0 } },
        started_{ start_stopwatch },
        start_{ start_stopwatch ? Clock::now() : stop_ }
    {
    }

    template <typename TimeUnit, typename Clock>
    inline void cr::stopwatch<TimeUnit, Clock>::start() noexcept
    {
        if ( started_ )
        {
            return;
            //throw std::logic_error( "stopwatch: already called start()" );
        }

        start_ = Clock::now();
        started_ = true;
    }

    template <typename TimeUnit, typename Clock>
    inline void cr::stopwatch<TimeUnit, Clock>::stop() noexcept
    {
        if ( !started_ )
        {
            return;
            //throw std::logic_error( "stopwatch: called stop() before start()" );
        }

        stop_ = Clock::now();
        stopped_ = true;
    }

    template <typename TimeUnit, typename Clock>
    inline TimeUnit cr::stopwatch<TimeUnit, Clock>::elapsed() noexcept
    {
        if ( !started_ )
        {
            return TimeUnit{ 0 };
        }

        if ( stopped_ )
        {
            return std::chrono::duration_cast<TimeUnit>( stop_ - start_ );
        }

        return std::chrono::duration_cast<TimeUnit>( Clock::now() - start_ );
    }

    template <typename TimeUnit, typename Clock>
    inline void
    cr::stopwatch<TimeUnit, Clock>::reset() noexcept
    {
        started_ = false;
        stop_ = start_;
        stopped_ = false;
    }

    template <typename TimeUnit, typename Clock>
    template <typename F, typename... FArgs>
    inline TimeUnit cr::stopwatch<TimeUnit, Clock>::measure( F&& f, FArgs&&... f_args )
    {
        auto start_time = Clock::now();
        std::forward<F>( f )( std::forward<FArgs>( f_args )... );
        auto stop_time = Clock::now();

        return std::chrono::duration_cast<TimeUnit>( stop_time - start_time );
    }
}
#endif

Sample tests:

#include <iostream>
#include <thread>
#include "stopwatch.h"

void f()
{
    std::this_thread::sleep_for( std::chrono::milliseconds( 250 ) );
}

int main()
{
    using namespace std::chrono_literals;

    cr::stopwatch<std::chrono::microseconds> sw{ true };
    std::this_thread::sleep_for( 100ms );
    std::cout << sw.elapsed().count() << '\n';

    std::this_thread::sleep_for( 100ms );
    sw.stop();
    std::cout << sw.elapsed().count() << '\n';

    std::cout << cr::stopwatch<>::measure( f ).count() << '\n';
}

Answer 1

What is a stopwatch?

My definition of a stopwatch is different than your implementation of a stopwatch. A stopwatch tracks the amount of time that has elapsed while the stopwatch is in an active state. The basic functionality is broken down into the following parts:

• start() - If not running, begin a new interval (update state and start time).
• stop() - If running, accumulate the current interval into the total elapsed time.
• elapsed() - If running, accumulate the current interval into the total elapsed time and begin a new interval.
• reset() - Set the stopwatch to its inactive state and wipe the total elapsed time.

In your implementation, you track the absolute time since the stopwatch construction or reset(). Your start() essentially acts as a reset() when stopped, thus making your stopwatch non-resumable. Consider the following example:

stopwatch<> sw(true);
std::this_thread::sleep_for(1s);
sw.stop();
std::this_thread::sleep_for(1s);
sw.start();
std::this_thread::sleep_for(1s);
auto elapsed = sw.elapsed(); 

What should be the value of elapsed?

Prefer non-member, non-friend functions

Functions should only be class members if they are required to be members (constructors, inherited functions, etc) or they need access to the internals to your class. Your measure() function is a candidate for being a free function and the implementation is roughly the same.

template <typename Callable, typename... Args>
auto measure(Callable&& func, Args... args) {
    cr::stopwatch<> sw{ true };
    func(std::forward<Args>(args)...);
    return sw.elapsed();
}

You can extend it to take any timer object (countdown timer, named timers, etc) if you wanted that functionality.

Use const consistently

Your implemented elapsed() function does not mutate the state of your stopwatch and can be marked as const.

Avoid latent usage errors by properly ordering headers

#include <iostream>
#include <thread>
#include "stopwatch.h"

While this is a small project and your includes are minimal, I would still recommend you order your includes. From John Lakos' "Large Scale C++ Software Design":

Latent usage errors can be avoided by ensuring that the .h file of a component parses by itself – without externally-provided declarations or definitions... Including the .h file as the very first line of the .c file ensures that no critical piece of information intrinsic to the physical interface of the component is missing from the .h file (or, if there is, that you will find out about it as soon as you try to compile the .c file).

To illustrate a clean and ordered header collection:

#include "stopwatch.h"                 // Prototype/Interface header
#include "my_graphics/graphics.hpp"    // Other headers in project
#include <boost/foreach.hpp>           // Non-standard headers
#include <boost/spirit/include/qi.hpp>
#include <iostream>                    // Standard headers
#include <vector>
#include <windows.h>                   // System headers

Avoid "magic" constants

int main() {
  using namespace std::chrono_literals;
  constexpr auto short_delay = 100ms;
  constexpr auto long_delay = 250ms;
  ...
}

Use symbolic constants as they offer semantic meaning to a statement, clearing up any confusion.

Prefer default arguments be assigned in declarations

class stopwatch {
    ...
    explicit stopwatch( bool const = false ) noexcept;
    ...
}

template <typename TimeUnit, typename Clock>
inline cr::stopwatch<TimeUnit, Clock>::stopwatch( bool const start_stopwatch /* = false */ ) noexcept
                                                                             ^^^^^^^^^^^^^

C++ Standard section 8.3.6 covers the rules regarding default arguments. From 8.3.6.4:

A default argument shall not be redefined by a later declaration (not even to the same value).

Since programmers and their tools work mostly from header files, prefer to assign default values at the function declaration site, and only assign to that parameter once.

Answer 2

Just a few comments on this one.

Only specify default parameters in the declaration

The constructor is already declared to have a default parameter value, so the implementation should omit the default value.

Remove state from the object

I don't see much point in using the state of the object only to ignore a user request, but that appears to be the sole use of the started_ member variable. If I happen to call start more than once, all calls after the first are simply ignored. For an alternative approach, see this stopwatch template.

Simplify the interface

It would be nice to be able to use the template like this:

std::cout << cr::stopwatch<>::measure(test) << '\n';

Unfortunately, that won't compile, and the user is required to tack on .count() to make this work. One cheesy but effective way to do this might be by use of a macro:

#define measure(x) #x << ": " << cr::stopwatch<>::measure(x).count() << " ms"

Now it can be used:

std::cout << measure(test) << '\n';

Sample output:

test: 191 ms

Answer 3

Simplify the interface

My suggestion: simply the interface. Instead of having to start() and stop(), and then separately having to keep track of whether or not they were called (which, btw, now your timing with stop() has to include checking !started_ - that's not nothing) - just drop all of that and have:

1. Construction starts the watch
2. A member gives you elapsed time

Which would be:

template <...>
class stopwatch {
public:
    stopwatch() noexcept 
    : start_(Clock::now())
    { }

    TimeUnit elapsed() const noexcept
    {
        auto stop = Clock::now();
        return std::chrono::duration_cast<TimeUnit>(stop - start_);
    }

private:
    TimeUnit start_;
};

This makes it easier to use your class and impossible to misuse. No silent failure of methods or anything, just:

stopwatch<...> timer;
// do stuff
auto elapsed = timer.elapsed();

Swap your template parameter ordering

The TimeUnit doesn't really matter - but the Clock does, so I would put it first:

template <class Clock = std::chrono::high_resolution_clock,
          class TimeUnit = std::chrono::milliseconds>
class stopwatch;

inline marking

Right now, you're putting inline on the definitions. I would put it on the declarations, since the user is primarily going to be looking at the interface:

template <...>
struct foo {
    inline void bar();
};

template <...>
void foo<...>::bar() { ... }

It doesn't matter which bar() you put inline on - it means the same thing either way, I just think it's more expressive this way.

Answer 4

Physical Stopwatches

From past experience of using physical stopwatches, I have found that when they have a separate start and stop function:

Start: Clicking the start multiple times merely restarted if it had already started.

Stop: Only stopped if it had already started.

---

Implementation

My personal opinion would be in the start and stop function just get the current time such that you do not add any time by checking the stopped_ and started_ Booleans (which is pretty insignificant).

Also, you could add a function say start_stop that most stopwatches have that will check a single Boolean on whether or not it has already been called. Therein a user can use that if they want a smarter start and stop if you want to. Then you could have the has_started function.

I will have to agree with @Edward on the added .count() to your library which would make a lot of people shy away from it because of the added step they must do. But then again, you do have it to where the methods return TimeUnit so the user should already know to do that.

---

MACRO

Just as a fun challenge, a while back I created a stopwatch library, but I did it through MACROs. That way I was able to get closer times to what the code was actually doing rather than having extra function calls. So, my measure function looked something like this:

#include <chrono>

// Used to store unique static variables prevent the 
// MACROs from having to create instances to hold the times.
template< unsigned ID, typename Clock >
struct TimeHolder { 
    static std::chrono::time_point< Clock > StartTime;
    static std::chrono::time_point< Clock > EndTime; 
};

#define STOPWATCH_MEASURE(ID, Clock, TimeUnit, CODE, RESULT)\
    TimeHolder< ID, Clock >::StartTime = Clock::now();\
    CODE;\
    TimeHolder< ID, Clock >::EndTime = Clock::now();\
    RESULT = std::chrono::duration_cast< TimeUnit >(\
        TimeHolder< ID, Clock >::StartTime - \
        TimeHolder< ID, Clock >::EndTime\
    ).count();

// Initializes the static variables.
template< unsigned ID, typename Clock >
std::chrono::time_point< Clock > TimeHolder< ID, Clock >::StartTime;

template< unsigned ID, typename Clock >
std::chrono::time_point< Clock > TimeHolder< ID, Clock >::EndTime;

Of course the code above is extremely watered down from what I have and some of the naming conventions I tailored closer to what you have.

---

Test Code

Here is some code I used to see how much the time was off of a one millisecond delay:

#include <chrono>
#include <unistd.h>

int main()
{
    {
        typedef std::chrono::microseconds TimeUnit;
        typedef std::chrono::high_resolution_clock Clock;
        std::chrono::time_point< Clock > start_time, stop_time;

        start_time = Clock::now();
        usleep(1000);
        stop_time  = Clock::now();

        std::cout << std::chrono::duration_cast< TimeUnit >( stop_time - start_time ).count() << std::endl;
    }
    {
        STOPWATCH_MEASURE(0, std::chrono::high_resolution_clock, std::chrono::microseconds, usleep(1000), int elapse)
        std::cout << elapse << std::endl;
    }
    {
        typedef std::chrono::microseconds TimeUnit;
        typedef std::chrono::high_resolution_clock Clock;
        std::cout << cr::stopwatch< TimeUnit, Clock >::measure(usleep, 1000).count() << std::endl;
    }
}

Running it for me, I would gain about ~40u without any library, ~45u using MACROs and about ~50u using your library. The time gained could be from a number of different things. So, these numbers probably are not a very good representation of what path is better by any means especially when we are looking at microseconds.