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 ?

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

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

Le 05/11/2011 19:33, Jonathan M Davis a écrit :
> ... - Jonathan M Davis

Thank you for your comprehensive response. It's very appreciated.
I am also interested in knowing why splitting the module was dropped.
As I said, it would make things a bit clearer, especially since the
sources embark the unit tests.
And a Ddoc page that spans a hundred screens long strikes me as unsound.

> Splitting off the formatting doesn't make sense, because it's part of
the  types.

OK.

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

I agree time zones without Systime is unlikely, but is it possible to use Systime without time zones, say for file timestamp for example ?

Dude

On Sunday, November 06, 2011 00:22:19 Somedude wrote:
> Le 05/11/2011 19:33, Jonathan M Davis a écrit :
> > ...
> > - Jonathan M Davis
> 
> Thank you for your comprehensive response. It's very appreciated.
> I am also interested in knowing why splitting the module was dropped.

In general, Phobos doesn't do submodules. It's a fllat hierarchy. We've changed that some recently in a few cases, but it's still mostly true. When std.datetime was introduced, it was decided that there was no need to split it up. It worked just fine as a single module. Splitting it into several sub- modules would have complicated things and went against how Phobos has generally been organized.

Any further discussions on splitting it up have come pretty much to the same conclusion with the addition that changing it now would break code, and the change just isn't worth it.

Beyond that though, I'd have to go digging through the archives to find what all of the specific reasons for not splitting it up were.

> As I said, it would make things a bit clearer, especially since the
> sources embark the unit tests.
> And a Ddoc page that spans a hundred screens long strikes me as unsound.

There would be just as much documentation if it were split up. The main problem IMHO is the fact that there is not a good way to navigate it (i.e. a proper set of links at the top that represent the hiearachy). So, instead of being able to easily hop to the type or function that you care about, you have to scroll or search for what you want. It becomes much harder to get an overview of the module. Proper links would go a _long_ way to fixing the problem. But until ddoc is improved, I can't do that. No one who has said that they were going to look into or work on the issue has actually gotten far enough to submit any fixes AFAIK.

> > 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.
> 
> I agree time zones without Systime is unlikely, but is it possible to use Systime without time zones, say for file timestamp for example ?

SysTime has member which is a timezone. Its design requires a TimeZone. It maintains its time internally in UTC (hecto-nanoseconds since midnight January 1st, 1 A.D. to be precise), and it converts that value to whatever the appropriate time zone is based on its timezone property. It avoids problems with DST that way. That's a core part of the design.

As for file timestamps, IMHO it would be very bad to have them without a timezone, unless they were just naked UTC. But inevitably, that leads to people trying to "adjust" that integral value to the local time or whatever time zone they want instead of leaving the time in UTC at all times (aside from when converting it to a string or asking for its year or whatnot). C does it that way, and it causes a number of problems. SysTime encapsulates it so that you don't have to worry about it. The time is always in UTC, and if you want to know the value in your local time zone, you ask it, and it uses its TimeZone to convert it to the appropriate value. Its internal time is never converted, so it avoids conversion errors.

- Jonathan M Davis

> Thank you for your comprehensive response. It's very appreciated. I am
> also interested in knowing why splitting the module was dropped. As I
> said, it would make things a bit clearer, especially since the sources
> embark the unit tests.
> And a Ddoc page that spans a hundred screens long strikes me as unsound.
> 
We should however thank Jonathan for the work he put into the documentation. It may be long, but it is thorough. Would you rather have the documentation from std.socket?

Steve

Le 06/11/2011 05:53, Steve Teale a écrit :
>> Thank you for your comprehensive response. It's very appreciated. I am
>> also interested in knowing why splitting the module was dropped. As I
>> said, it would make things a bit clearer, especially since the sources
>> embark the unit tests.
>> And a Ddoc page that spans a hundred screens long strikes me as unsound.
>>
> We should however thank Jonathan for the work he put into the documentation. It may be long, but it is thorough. Would you rather have the documentation from std.socket?
> 
> Steve
Yes, of course, I'm conscious of the tremendous effort put in this module and its documentation, there is no question about it, and really I didn't mean to underestimate it.

Le 06/11/2011 02:35, Jonathan M Davis a écrit :
> On Sunday, November 06, 2011 00:22:19 Somedude wrote:
>> Le 05/11/2011 19:33, Jonathan M Davis a écrit :
>>> ...
>>> - Jonathan M Davis
>>
>> Thank you for your comprehensive response. It's very appreciated.
>> I am also interested in knowing why splitting the module was dropped.
> 
> In general, Phobos doesn't do submodules. It's a fllat hierarchy. We've changed that some recently in a few cases, but it's still mostly true. When std.datetime was introduced, it was decided that there was no need to split it up. It worked just fine as a single module. Splitting it into several sub- modules would have complicated things and went against how Phobos has generally been organized.
> 
> Any further discussions on splitting it up have come pretty much to the same conclusion with the addition that changing it now would break code, and the change just isn't worth it.
OK. I agree that a flat hierarchy is generally better than a nested one, although 2 levels is still not too bad (provided that std is the root).
> Beyond that though, I'd have to go digging through the archives to find what all of the specific reasons for not splitting it up were.
> 
>> As I said, it would make things a bit clearer, especially since the
>> sources embark the unit tests.
>> And a Ddoc page that spans a hundred screens long strikes me as unsound.
> 
> There would be just as much documentation if it were split up. The main problem IMHO is the fact that there is not a good way to navigate it (i.e. a proper set of links at the top that represent the hiearachy). So, instead of being able to easily hop to the type or function that you care about, you have to scroll or search for what you want. It becomes much harder to get an overview of the module. Proper links would go a _long_ way to fixing the problem. But until ddoc is improved, I can't do that. No one who has said that they were going to look into or work on the issue has actually gotten far enough to submit any fixes AFAIK.
I understand and I agree.

>>> 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.
>>
>> I agree time zones without Systime is unlikely, but is it possible to use Systime without time zones, say for file timestamp for example ?
> 
> SysTime has member which is a timezone. Its design requires a TimeZone. It maintains its time internally in UTC (hecto-nanoseconds since midnight January 1st, 1 A.D. to be precise), and it converts that value to whatever the appropriate time zone is based on its timezone property. It avoids problems with DST that way. That's a core part of the design.
> 
> As for file timestamps, IMHO it would be very bad to have them without a timezone, unless they were just naked UTC. But inevitably, that leads to people trying to "adjust" that integral value to the local time or whatever time zone they want instead of leaving the time in UTC at all times (aside from when converting it to a string or asking for its year or whatnot). C does it that way, and it causes a number of problems. SysTime encapsulates it so that you don't have to worry about it. The time is always in UTC, and if you want to know the value in your local time zone, you ask it, and it uses its TimeZone to convert it to the appropriate value. Its internal time is never converted, so it avoids conversion errors.
> 
> - Jonathan M Davis
Thank you for your explanation.

Dude

On Sunday, November 06, 2011 09:53:26 Somedude wrote:
> Le 06/11/2011 05:53, Steve Teale a écrit :
> >> Thank you for your comprehensive response. It's very appreciated. I am
> >> also interested in knowing why splitting the module was dropped. As I
> >> said, it would make things a bit clearer, especially since the sources
> >> embark the unit tests.
> >> And a Ddoc page that spans a hundred screens long strikes me as
> >> unsound.
> > 
> > We should however thank Jonathan for the work he put into the documentation. It may be long, but it is thorough. Would you rather have the documentation from std.socket?
> > 
> > Steve
> 
> Yes, of course, I'm conscious of the tremendous effort put in this module and its documentation, there is no question about it, and really I didn't mean to underestimate it.

The particularly frustrating thing is that it really isn't all that complicated. It's just that there's a lot of it, and so it appears daunting. I eagerly await the time when I'll be able to improve the links at the top, which I really think will help make the module more accessible.

In any case, if you haven't read it yet, you really should read this article: http://d-programming-language.org/intro-to-datetime.html

It should help clarify std.datetime and will probably answer a number of the questions that you might have.

- Jonathan M Davis

Le 06/11/2011 09:00, Jonathan M Davis a écrit :
> In any case, if you haven't read it yet, you really should read this article: http://d-programming-language.org/intro-to-datetime.html
> 
> It should help clarify std.datetime and will probably answer a number of the questions that you might have.
> 
> - Jonathan M Davis

Thanks.

Dude