std.datetime is too big

[Alex Rønne Petersen]

On 05-11-2011 19:33, Jonathan M Davis wrote:
> On Saturday, November 05, 2011 08:58:27 Somedude wrote:
>> Hello,
>>
>> I've been lurking for a while here and I really appreciate all the work
>> that is done on D and Phobos. This is a fantastic language and a great
>> little community. So I don't want the following comments to be badly
>> perceived and I hope they will be seen as constructive.
>>
>> Browsing through the source code of Phobos, I "stumbled" upon the huge
>> block that is std.datetime. Now I know that know that date/time is a
>> very complex subject, and that this module has been written by Jonathan
>> Davis, who is one of the smartest guys out there, so I am certainly not
>> going to comment on the implementation.
>>
>> However, I believe the interface is way too big at the moment, and would
>> highly benefit from being broken up into more focused modules. Just
>> looking at the std.datetime Ddoc is a pain, let alone understanding how
>> to do fairly simple tasks.
>>
>> For me, the module contains way too many disparate concepts to be usable
>> at the moment:
>> - time zones
>> - calendars
>> - time intervals and ranges over intervals
>> - formatting and validation
>> - stopwatch
>> - benchmark
>>
>> It looks that everything time related has been crammed in this single
>> module. It makes it unnecessarily hard to understand what to choose and
>> how to use it. Also,>34,000 LOC makes the code nearly impossible to
>> review and help/correct for people who are not the authors.
>>
>> I would suggest breaking this module into submodules per concept or
>> activity, maybe something like:
>> std.datetime.timezone
>> std.datetime.calendar
>> std.datetime.interval
>> std.datetime.format
>> and I would entirely remove everything related to stopwatch and
>> benchmarking for another module.
>>
>> What do you think ?
>
> The stopwatch and benchmarking stuff will likely end up in std.benchmark.
> Andrei has such a module in the works which - assuming that it passes the
> review process - does include moving that functionility to it.
>
> Splitting off the formatting doesn't make sense, because it's part of the
> types.
>
> Splitting of the time zones could be done, but I don't know why anyone would
> ever use them without SysTime, so I don't see much point in putting them in
> another module.
>
> The interval and range stuff could definitely be put in a separate module
> (particularly since they just use the various time point types - Date,
> TimeOfDay, DateTime, and SysTime - rather than sharing any implementation with
> any of them). So, at one point, I did suggest that we split that off, and it
> was decided that we wouldn't. Any and all suggestions that std.datetime be
> split up has been shot down by the Phobos devs.

Out of curiosity, what were the arguments against doing so?

>
> The only major change with regards to size that many of them were for is
> reducing the number of lines that the unit tests take up. In the interest of
> making the unit tests less likely to be wrong, I made them very simple and
> used pretty much no loops of any kind, and some folks didn't agree with that
> way of writing unit tests - particularly since it results in a lot of lines of
> code when you're thorough about testing. So, I've slowly been rewriting them
> so that they use loops and the like, which should reduce the length of the file
> but have no effect on what functionality is in it.
>
> The number one thing that _I_ would like to see which would make std.datetime
> much easier to digest would be for ddoc to be fixed so that the anchors that it
> generates actually represent the code's hierarchy. For instance, right now,
> the year functions for Date, DateTime, and SysTime all get the exact same
> anchor - #year - so you can't link to each individual function.  It needs to
> be generating anchors more along the lines of #Date.year, #DateTime.year, and
> #SysTime.year. That way, I could organize the links at the top of the
> documentation and make it so that they're actually informative and help you
> understand the module instead of confusing you.
>
> - Jonathan M Davis

- Alex