datetime review part 2 [Update 4]

[Jonathan M Davis]

On Wednesday 10 November 2010 10:18:27 Dmitry Olshansky wrote:
> 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).

I'm not sure the best way to do all of that. std.container tries to give a 
general overview of container capabilities at the beginning and then still has 
all of the specific functions listed for each type still. I could try something 
like that, though making that sort of change is not something that I'm likely to 
get to very quickly. It'll take a fair bit of thinking and experimentation to 
come up with the best way to do that. As for specific overloads and 
documentation, I'm not sure what the best way to handle that is, but it does 
need to be made clear which functions work with which types. Taking the interval 
functions for instant, some of them - like intersection - work with all interval 
types regardless of the interval type that you're dealing with, while others - 
like union - only work with a subset of the interval types (since things like 
the union between positively and negatively infinite intervals make no sense).

What I likely need to do is try and come up with better documentation up front 
which better gets across the basic groups of types (time points, intervals, 
etc.) and their basic abilities while leaving the individual documentation to 
still give all of the details that it needs to. I'll look into doing that later, 
but I can't say that it's a high priority at the moment. Good suggestion though.

> 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

I'm afraid that I don't really get what you're trying to do here. A range needs 
to be created from an interval. It really wouldn't make sense to do it 
otherwise. And when you create a range, it needs a delegate which generates the 
time points for that range. So, you create an interval and call either 
fwdRange() or bwdRange() on it to give it the range-generating delegate. I 
happen to have provided some helper functions for generating delegates, but 
there's nothing to stop you from creating your own.

Are you trying to say that the range should automatically provide _all_ possible 
time points in an interval and then you specifically filter them? That's nowhere 
near as flexible, and in the case of SysTime in particular, think about how many 
time points that is. It has hecto-nanosecond (100 ns) precision.  That's 10 
million time points a second. It could get really inefficient to try and filter all 
of those. Also, I'm not sure that filter would work with an infinite range (I'd 
have to check), which would be a huge setback. I really don't get the benefit of 
your suggestion, though I can undkerstand it if it's not entirely clear how 
ranges in std.datetime are supposed to work.

Essentially, you provide a function that takes a time point and returns the next 
time point in the range. It's a generative function, successively generating 
each time point in the range as you iterate. It makes it really easy to create 
infinite ranges and is completely flexible as to how time points are generated, so 
you can generate essentially any kind of range over an interval of time points 
as long as each time point in the range is later (or earlier if iterating 
backwards) than the previous one. It's kind of like having a range over some 
subset of the fibonacci sequence. Each iteration causes the generative function 
to generate the next number/time point in the sequence. If you do something like 
take(range, 5), then it generates a range of 5 points, and if you use it 
directly in a foreach loop, then it iterates over them all (which in the case of 
fibonacci or an infinite range in datetime would go on forever, but in the case of 
a non-infinite range from datetime would be until the end of the interval).

I think that what I came up with is extremely flexible and generic, and I think 
that it's quite good, if perhaps a bit confusing at first. I think that you're 
the first to comment on it though.

> 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]

Well, I expect that I'm lucky if those are the only typos. Thanks for pointing 
them out. They've now been fixed in my local copy.

- Jonathan M Davis