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
