std.datetime is too big

Jonathan M Davis jmdavisProg at gmx.com
Sat Nov 5 11:33:52 PDT 2011


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.

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


More information about the Digitalmars-d mailing list