datetime review part 2 [Update 4]

[Dmitry Olshansky]

On 10.11.2010 4:52, Jonathan M Davis wrote:
> On Tuesday, November 09, 2010 17:34:15 Jonathan M Davis wrote:
>> On Tuesday, November 09, 2010 16:23:56 Yao G. wrote:
>>> On Tue, 09 Nov 2010 18:11:59 -0600, Jonathan M Davis
>>> <jmdavisProg at gmx.com>
>>>
>>> wrote:
>>>> I think that the real question here is how good the API is.
>>> I think that the sheer size of the library, specially with a datetime.d
>>> file with almost 31,600 lines of code (granted, most of them are unit
>>> test and documentation), makes a little difficult to analyze or give a
>>> proper review of the code. It will takes a fair share time to do it.
>>> Maybe that's why very few people has given criticism or suggestions. I
>>> only gave a cursory view (you need to scroll a lot just to find the
>>> implementation code), but I'll give it a more throughly review tonight.
>> That's quite understandable, but that's part of the reason that the ddoc
>> html files are there. You don't actually have to go trolling through the
>> code to look at the API.
> I should probably add that it would be worth it for folks to just look at the
> documentation for what they would generally try and do with a datetime module
> and see whether they can do it reasonably well and what problems they'd have.
> While looking at the module as a whole is definitely needed, just having folks
> look at the portions that they'd use and commenting on that would be useful. Is
> getting the time easy enough and intuitive enough? Is doing what you need to do
> with time in basic applications once you have it easy or intuitive enough? Etc.
>
> - Jonathan M Davis

OK, I looked through the docs, to me it's very solid. I fact I never 
written any code that heavily uses date-time stuff, most of the time I 
just converted everything to UNIX timestamp, and back to printable form, 
once the processing is done.

One suggestion about docs - since SysTime and DateTime have similar API, 
wouldn't it be better to merge documentation
and represent it in form of a table (like the one in std.container, 
unfortunately ATM  it's screwed up )?
  In the same table you could also place functions specific for SysTime, 
just add a remark to it.
Another one - group overloads together, consider for instance std.algorithm.
It doesn't leak all of it's template specializations for all kinds of 
ranges with the same ddoc comment.  You shouldn't i.e.
const pure bool_intersects_(in Interval/interval/);
Whether the given/interval/overlaps with this/interval/.
const pure bool_intersects_(in PosInfInterval!(TP)/interval/);
Whether the given/interval/overlaps with this/interval/.
...
It only could get you tired - there is no new information at all.
May I also humbly suggest you create the second table describing generic 
Interval notion (with specifics fro infinite ones marked as such).

And one more thing about the _API_ itself:
auto interval = Interval!Date(Date(2010, 9, 2), Date(2010, 9, 27));
auto func = IRange.everyDayOfWeek!Date(DayOfWeek.monday);
auto range = interval.fwdRange(func);

I'd preffer more simple and generic things, they tend to combine with 
std.algorothm so nicely (It's just an idea worth researching).
IMHO also could remove a ton of code from date-time, that do not belong 
here anyway.
My try at the code above would be:
auto interval = Interval!Date(Date(2010, 9, 2), Date(2010, 9, 27));
auto onlyMondays = filter!((x){ return x.dayOfWeek == DayOfWeek.monday });
auto range = onlyMondays (interval.by!"days"); //reuses the same 
convention for add and roll, more coherent

P.S. [Nitpicking]

Some typos in docs:
int_diffMonths_(in Date/rhs/);
     ... You can get the difference in _yours_ by subtracting the year 
property of two Dates,...
Should be "years"? Same thing in SysTime and DateTime, that just proves 
my earlier point - documentations for these could be merged.
---
struct_SysTime_;
_    SysTime_is the type used when you want _to_ the current time from 
the system or if you're doing anything that involves time zones...
    Should be "to get"?

[/Nitpicking]

-- 
Dmitry Olshansky