datetime review part 2 [Update 4]

[Dmitry Olshansky]

On 11.11.2010 8:55, Jonathan M Davis wrote:
> 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.
If you have provided them, then it's OK.
What I meant is we have retro(someRange) to go backwards where applicable.
  And I do believe you could return range of DateTime's  that's 
bidirectional ?
Then there is no need to duplicate the std.algorithm/range 
functionality, that's all.
> 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.
Not at all, you provide the precision that the user asks for. E.g. if 
users want tot iterate by days - no problem iterate by days.
If by hnseconds, well he(or she) knows better ... Right?
The point is we don't need some predefined delegates, just the natural 
ones e.g. by secs, hours and so on as with the roll/add methods.
> 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).
No problem at all, though I didn't check if std.range really supports 
infinite ranges, though it should.
> 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.
>
Well, I'm not saying it's bad/or some such. I, first of all, told you 
it's damn good.
Things could be simplified though if you relay some of burden on 
std.algorithm/range and such.
That's the main point I couldn't get for the moment.
>> 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


-- 
Dmitry Olshansky