[phobos] Proposal of StopWatch module

[Andrei Alexandrescu]

On 08/18/2010 12:21 AM, Jonathan M Davis wrote:
> On Tuesday 17 August 2010 21:46:27 SHOO wrote:
>>> 2. As a user of this module, I would much prefer to have an exception
>>> throw when I call start or stop function out of order, instead of silent
>>> return. If the only returns, there semantic requirement for having stop
>>> function - it is has then same meaning as peek.
>>
>> There are three ways:
>> 1. enforce
>> 2. assert
>> 3. return;
>>
>> Which do you like?
>> I like #2 because I want to suppress influence to runtime.
>
> I think that the typical thing to do at this point is to use assert internally
> to verify the state of the struct or class itself, and to use enforce to check
> input from the users of the module.

I agree. Here is a brief code review, I'm basing it on 
http://ideone.com/TVw1P:

Line 21: This "struct" trick is interesting, why are you using it? Also, 
I recommend type names to start with a capital (not vital because this 
is private anyway).

Line 47 and beyond: shouldn't we use at best use ulong instead of long?

Line 94 and others: you have a fair amount of @system methods that are 
very @safe. Why mark them as @system? I think the entire module is @safe 
in fact.

Line 212: So far you used seconds/fromSeconds, useconds/fromUseconds 
etc. Now you have interval() which does not inform about the unit of 
measurement. How about exactSeconds or realSeconds? Speaking of which, 
I'm not sure if anyone would be ever interested in the truncated 
seconds, so why not just dump that guy and then rename interval() to 
seconds()? To further clarify: if someone cares only about truncated 
seconds, it's unlikely they're also performance-worried enough to not 
work with floating-point numbers (besides, heck, FP division is actually 
much faster than integral division).

Line 229-240: consolidate these two operators with a mixin.

Line 265-284: same thing

Line 318: return cast(int) (value - t.value);

Line 335-359: consolidate with mixin

Line 376-389: same thing

Line 414: no need for @trusted, casting numbers is not unsafe.

Line 438: Please don't give examples using the scope keyword; it is well 
on its way to deprecation.

Line 461: it would be difficult to justify the design choice of making 
this as a class. First, it has methods that I don't see how anyone can 
override gainfully: objectTime(), reset(), start(), stop(), peek(). 
These are quite clear pieces of functionality. Why would anyone override 
them? Second, if you're measuring exact timings, you're very performance 
sensitive. Inserting an object allocation in the middle of everything is 
overkill. I think all we need is a simple, cheap stack type similar to 
Ticks.

Line 489: if we go with a struct design, we should have a constructor 
that "autostarts" a StopWatch, e.g.:

auto myWatch = StopWatch(AutoStart.yes);

Line 637: QueryPerformanceCounter's return value is ignored. Please make 
a pass through all code and make sure you check all return values.


Nice work!

Andrei