I'd also like you and other participants to try answering to points marked with "gps" in the source code.
------------------------------------------------------------------------------ // gps - temporary helper void global_throw_helper(const char* msg) { throw std::runtime_error(msg); } We'd just use boost::throw_exception here. ------------------------------------------------------------------------------ template <typename T> struct clock_device_traits { // gps TODO //has_synch_start? }; I think maybe we'd have a clock_device_base that is inherited by every clock_devices, that would be templated on the time type and typedefs time_duration_type, so the elapsed timer that accepts a clock_device can also typedef those. ------------------------------------------------------------------------------ // gps - do we need these two typedefs? // typedef TimePoint time_type; // typedef typename Device::time_point_type time_point_type; I think it's convenient, I even propose we do : typedef ClockDevice clock_type; typedef typename ClockDevice::time_type time_type; typedef typename time_type::time_duration_type time_duration_type; So the user can know the clock_type for when he uses a convenience typedef like "typedef timer<microsec_device> microsec_timer;" which is likely to be declared in the library. ------------------------------------------------------------------------------ void resume(); // gps - what if you call twice pause, twice resume consecutively?? // bool is_running() const // gps - add this?? // gps: I see no point in having clear_elapsed() I think calling twice pause or resume should have no effect. Not doing so is too error prone imho. is_running() sounds like good idea tho I can't really think of a use for it so maybe not if that means having to add a data member to implement it. I think reset() was good, why don't you see a point for it ? If you want to time two consequent things individually and use the same object ? ------------------------------------------------------------------------------ template <typename D> void elapsed_timer<D>::start() // may throw { restart(); // this is just the same as restart(); // maybe we can come up with a common name -gps } I think calling start() twice should not reset it, I think restart should call reset() then start(). ------------------------------------------------------------------------------ template <typename D> void elapsed_timer<D>::pause() { // throw if(!is_running) ?? - gps duration += device.elapsed(); is_running = false; } Again, I think pause should pause if not already paused, otherwise do nothing, like the implementation I uploaded based on Jeff's design. ------------------------------------------------------------------------------ // Specific Clock Devices // gps - should this be copyable? Why not ? Anyway, I'll skip the other points as it's more implementation detail it seems, but I'll explain more how I think the timer should behave : - Calling start() should start if not already started, subsequents calls to start() do nothing. - Calling pause() should pause if not already paused, subsequents calls to pause() do nothing. - Calling resume() should start the timer again at the time duration the timer was if not already start, subsequents calls to resume() do nothing. - Calling reset() should reset the current elapsed time to 0. - Calling restart() should call reset() then start(). - Calling elapsed() should return the current timer value. I also think your interface lacks the hability to set the initial time offset, which could be usefull in some situations. Ok, it's a start ! I'll see if I can comment on the other gps points in a near future. Philippe