On Tue, 18 Jul 2006 12:10:56 +0200, "Philippe Vaucher" <philippe.vaucher@gmail.com> wrote:
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.
I'm afraid I didn't make my point clear. It wasn't about supporting BOOST_NO_EXCEPTIONS (which I'd like not to --it would require in any case a re-examination of the code, as I didn't design it with that in mind). It was to discuss what type of exception(s) we want to throw and what additional info, if any, we want to add to exception objects. Maybe Boost.Date-Time already has a well thought-out solution.
------------------------------------------------------------------------------ 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.
I'm not sure we are understanding each other. The main advantage of my solution is that it doesn't logically imply/require any usage of a "time point". That means: if you measure while someone (or some program/service) adjusts the system clock, or a DST switch happens, you are *not* affected. Example: you begin measuring at 2am of your local time, in the day when DST is switched off. Your test takes 1h 30m. With your calculation you get: start = 2am, elapsed = 0 end = *2:30am*, elapsed = 30 min Your approach would work if date_time offered a concept of time-zero point: a special time point value which would only be used to calculate differences, so to elide it automatically: (stop-t0) - (start - t0) = stop-start Such a value could be used as implementation detail. But frankly I don't see a reason for it from a logical point of view. We only care about durations and durations are addable; that's all we need.
------------------------------------------------------------------------------ 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.
Maybe.
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.
Basically you are saying you are in doubt. I was too, hence my question (incidentally: what's your concern with an additional data member??).
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 ?
restart()?
------------------------------------------------------------------------------ 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().
Honestly, did you really look at my code? Looks like you keep thinking in terms of yours. I had no concept of reset() at all.
------------------------------------------------------------------------------ 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.
I see. But the point is: if the user code happens to call pause() twice without intervening restarts, is that a sign of a logical error? It's similar to set a pointer to null after a delete, to ensure a double deletion is harmless. It may make you feel safer, but in practice it hides programming errors by eating their immediate ill effects. I may see a use for is_running at least in debug mode here.
------------------------------------------------------------------------------ // Specific Clock Devices // gps - should this be copyable?
Why not ?
It's a choice, and I annotated it to make sure I gave it another thought at the end of the work.
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.
Same comment as above.
- Calling pause() should pause if not already paused, subsequents calls to pause() do nothing.
Again.
[...]
I also think your interface lacks the hability to set the initial time offset, which could be usefull in some situations.
Care to explain?
Ok, it's a start !
Yes, thanks. At least it should clear some manifest misunderstanding. -- [ Gennaro Prota, C++ developer for hire ] [ resume: available on request ]