9
\$\begingroup\$

I needed to implement a stop watch like class that will be used to measure running time of different functions by others. The final running time needs to be printed in 9h 3min 2s 23s 32ms 87us 78ns format. If the larger units are zero then they must be omitted. Time duration in a particular time-unit like seconds or milliseconds can be obtained from functions similar to hours. I need better ideas to improve the implementation.

#ifndef _STOP_WATCH_HPP_
#define _STOP_WATCH_HPP_

#include <chrono>
#include <utility>
#include <iostream>

class stop_watch {
    std::chrono::high_resolution_clock::time_point last_time_point;
    std::chrono::duration<double> time_duration;
    bool is_running;
  public:
    stop_watch() : 
      last_time_point {std::chrono::high_resolution_clock::now()},
      time_duration {std::chrono::duration<double>::zero()},
      is_running {false}
      {}
    void start() {
      assert(!is_running); // stop watch is already running
      last_time_point = std::chrono::high_resolution_clock::now();
      time_duration = std::chrono::duration<double>::zero();
      is_running = true;
    }
    void stop() {
      assert(is_running); // stop watch is not running
      auto n = std::chrono::high_resolution_clock::now();
      time_duration = n - last_time_point;
      last_time_point = n;
      is_running = false;
    }
    double hours() {
      return std::chrono::duration_cast<std::chrono::hours>(
          time_duration).count();
    }
    friend std::ostream& operator<<(std::ostream& out, stop_watch sw) {
      std::chrono::hours h = std::chrono::duration_cast<
          std::chrono::hours>(sw.time_duration);
      if (h.count()) {out << h.count() << "h "; sw.time_duration -= h;}
      std::chrono::minutes m = std::chrono::duration_cast<
          std::chrono::minutes>(sw.time_duration);
      if (m.count()) {out << m.count() << "min "; sw.time_duration -= m;}
      std::chrono::seconds s = std::chrono::duration_cast<
          std::chrono::seconds>(sw.time_duration);
      if (s.count()) {out << s.count() << "s "; sw.time_duration -= s;}
      std::chrono::milliseconds ms = std::chrono::duration_cast<
          std::chrono::milliseconds>(sw.time_duration);
      if (ms.count()) {out << ms.count() << "ms "; sw.time_duration -= ms;}
      std::chrono::microseconds us = std::chrono::duration_cast<
          std::chrono::microseconds>(sw.time_duration);
      if (us.count()) {out << us.count() << "us "; sw.time_duration -= us;}
      std::chrono::nanoseconds ns = std::chrono::duration_cast<
          std::chrono::nanoseconds>(sw.time_duration);
      if (ns.count()) {out << ns.count() << "ns "; sw.time_duration -= ns;}
      return out;
    }
};

#endif

Sample usage

stop_watch sw;
sw.start();
// function(s) to measure time
sw.stop();
cout << "It took " << sw << endl;

I need some guideline on how to do this more efficiently, as my code is already too messy :(. I also have the following questions:

  1. Is there a way to generate the functions for hours, seconds, etc. dynamically?
  2. How can I improve the operator<< function? Is there a way to have a list of time units that I can iterate?
\$\endgroup\$
  • \$\begingroup\$ Are you sure you want C++11 only? It is 7 years old now. \$\endgroup\$ – Incomputable Feb 4 '18 at 10:05
  • \$\begingroup\$ @Incomputable, it has to be in c++11 :(. \$\endgroup\$ – alexwest Feb 4 '18 at 10:24
  • 1
    \$\begingroup\$ It seems like people don't like the parts Additional functions are needed to get the duration in a particular timeunit like seconds or milliseconds. I have just added one of them (i.e., hours). and Is there a way to generate the functions for hours, seconds, etc. dynamically?, which kind of makes suggests the code is broken or incomplete. It would be great to remove them or reword to stress on improvement rather than creation. It is just my guess, the post seems fine for me. \$\endgroup\$ – Incomputable Feb 4 '18 at 10:45
  • \$\begingroup\$ @Incomputable, updated. let me know whether it is better now? \$\endgroup\$ – alexwest Feb 4 '18 at 10:52
  • \$\begingroup\$ Seems better for me now. Also, answers here usually take some time. The longest I've seen yet was 7 days excluding necromancy. The average though is less than one day. \$\endgroup\$ – Incomputable Feb 4 '18 at 10:55
11
\$\begingroup\$

Misc.:

  • Modern compilers all support #pragma once. It's much cleaner than old-school include guards.
  • You need a #include <cassert>

Code Improvements - Class:

  • Use typedefs to reduce code duplication (and if you ever need to change the clock type, it's much simpler). These should be public.

    using clock_t = std::chrono::high_resolution_clock;
    using duration_t = clock_t::duration;
    
  • Store the duration type at the resolution of the clock, rather than using duration<double>.

    clock_t::time_point last_time_point;
    duration_t time_duration;
    
  • The hours function should be const, but I'd actually recommend replacing it with a function that returns the time in the most accurate units. The stopwatch class shouldn't worry about time conversions - only the user code knows the desired accuracy, and can duration_cast if necessary:

    duration_t get_elapsed_time() const
    {
        return time_duration;
    }
    
  • Since the code asserts on the running state in start() and stop(), we probably need a function to check if the stopwatch is running:

    bool get_is_running() const
    {
        return is_running();
    }
    

Code Improvements - Output:

  • Now that we have get_elapsed_time(), the ostream operator doesn't need to be a friend.

  • Use auto here too.

  • Doing the printing separately from extracting each unit of time makes things clearer.

  • The ostream operator should take the stopwatch by const&, not by value. (I doubt it makes much difference performance-wise, but it makes the intent and semantics clearer - surely we shouldn't need to copy the stopwatch object to print the time?). Outsourcing the printing to a separate function also makes things a bit neater (more lines of code, but less duplication). It could be a lambda function if you wanted.

    namespace detail
    {
    
        template<class duration_t>
        void print_duration_if_non_zero(std::ostream& out, duration_t const& duration, std::string const& unitPostfix)
        {
            if (duration == duration_t::zero())
                return;
    
            out << duration.count() << unitPostfix;
        }
    
    } // detail
    
    std::ostream& operator<<(std::ostream& out, stop_watch const& sw) {
    
        auto t = sw.get_elapsed_time();
    
        auto h = std::chrono::duration_cast<std::chrono::hours>(t); t -= h;
        auto m = std::chrono::duration_cast<std::chrono::minutes>(t); t -= m;
        auto s = std::chrono::duration_cast<std::chrono::seconds>(t); t -= s;
        auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(t); t -= ms;
        auto us = std::chrono::duration_cast<std::chrono::microseconds>(t); t -= us;
        auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(t);
    
        detail::print_duration_if_non_zero(out, h, "h ");
        detail::print_duration_if_non_zero(out, m, "min ");
        detail::print_duration_if_non_zero(out, s, "s ");
        detail::print_duration_if_non_zero(out, ms, "ms ");
        detail::print_duration_if_non_zero(out, us, "us ");
        detail::print_duration_if_non_zero(out, ns, "ns ");
    
        return out;
    }
    
  • It would be cleaner not to give stop_watch an ostream operator at all. If you create a print_time_to_stream(stream, time) that takes time as std::chrono::nanoseconds (or whatever's the highest resolution you need in your app), then your print function can be used for any time, and not just for the stop_watch class!

Your Questions

  1. get_elapsed_time() makes this unnecessary! User code should convert the duration when it needs to.
  2. Since std::chrono::hours etc. are types, they can't just be added to a vector. I suppose you could do something like:

        auto times = std::vector<stop_watch::duration_t::rep>();
        times.push_back(std::chrono::duration_cast<stop_watch::duration_t>(std::chrono::hours(1)).count());
        // same for other desired types...
    
        auto units = std::vector<std::string>();
        units.push_back("h ");
        // ...
    
        print_times(std::cout, sw.get_elapsed_time(), times, units);
    

    So the times vector contains the time to use in the units of stop_watch::duration_t. And then you can use integer division to get the necessary values instead of duration_casting.

Other Things

If you're just measuring function times, you might not need the start / stop logic. You could perhaps simplify the class to something like the following:

class timer
{
public:

    using clock_t = std::chrono::high_resolution_clock;
    using duration_t = clock_t::duration;

private:

    clock_t::time_point start_time;

public:

    timer():
        start_time(clock_t::now()) { }

    duration_t get_elapsed_time() const { return (clock_t::now() - start_time); }

    void reset() { start_time = clock_t::now(); }
};

There's no need to worry about leaving the timer "running", since there's no actual work unless you query the time...

timer sw;
std::this_thread::sleep_for(std::chrono::seconds(1));
std::cout << "It took: " << sw.get_elapsed_time().count() << "units" << std::endl;
\$\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.