Re: [boost] [date_time] improvements

From: Jeff Garland (jeff_at_[hidden])
Date: 2003-08-04 08:33:24

On Mon, 4 Aug 2003 12:50:36 +0300, John Torjo wrote

> Told you I'd come back for more ;)
> Here are some more improvements I would consider useful:
> 
> [1]
> unary operator-(time_iterator).
> Example: -hours(24) instead of hours(-24).
> (seems more straightforward)

I see your point, but then don't you have to add all the other
operators for consistency?  Not sure that makes sense to do with
the iterator.
 
> [2]
> Does a *FOUR* functions justify having a jam file?
> (greg_month::as_short_string(), greg_month::as_long_string(),
>  greg_weekday::as_short_string(), greg_weekday::as_long_string())
> 
> They could definitely be made static or something.
> (or the user could be given a choice - using the date_time from a
> .jam-generated
>  dynamic link library, or directly)

Yes, to avoid multiple definitions of the said strings without having 
to recreate them every time a date is formatted.  Also, if a set of
localized strings ever gets into the library there will be many more
strings (take a look at libs/date_time/test/gregorian/test_facet.cpp).
Finally, there may be other functions that eventually will be in the 
library and not inlined. 

BTW, my original intent was to make much more of the library 'inline
swappable'. That is, by flipping a compile switch large chunks of the
library could either be inline or in the library -- whichever was 
better for the user's performance / space tradeoff.  I've let this
slide for now as the way I did it was a pain for users and there are
more important issues at the moment...

> [3]
> I've been recently involved in a project that used to use raw time_t's.
> A conversion to/from time_t would come in very handy.
> (at least one way would be enough).
> I've created such a function:
> 
> #include <boost/date_time/posix_time/posix_time.hpp>
> 
> // converts raw time to cool ptime structure
> inline boost::posix_time::ptime rawtime_to_ptime( time_t raw) {
>     using namespace boost::posix_time;
>     using namespace boost::gregorian;
> 
>     tm details = *localtime( &raw);
>     date day( details.tm_year + 1900, details.tm_mon + Jan,
> details.tm_mday);
>     time_duration offset( details.tm_hour, details.tm_min, 
> details.tm_sec, 0);    return ptime( day, offset); }
> 
> pretty simple and straightforward, I think.

I agree, this is on the todo list to add to the library. See
below for why it isn't already.  I think I would write it like 
this, however:

//Warning -- off the top of my head untested code (but should work)!
//in boost::posix_time namespace 
posix_time from_time_t(time_t t) {
  using namespace boost::gregorian;
  return ptime(date(1970,1,1), seconds(t));
}

> [4]
> time_duration::fractional_seconds() seems pretty confusing.

Sorry about that...

> As I understood by looking at the docs, fraction actually means
> nano-seconds.

Not necessarily.  It depends on the resolution the time duration template is
instantiated with.  For example, there is a second option that many people use
which only provides microsecond duration resolution, but only requires a 
single 64 bit integer to represent a time value.  The bottom line is that
fractional seconds is a count of the number of fractional seconds at the
given resolution.
 

> [5]
> In file gregorian_calenar.hpp, you
> #include "gregorian_calendar.ipp"
> 
> I think to make it more portable, it should be:
> #include <boost/date_time/gregorian/gregorian_calendar.ipp>

Yep, that should be fixed.

> [6]
> documentation - does not say if greg_day starts from one or zero
> (from experiments, I realized it starts from one)
> Maybe this is not so important, but was confusing for me at first.

Well, I think greg_day might be a bit under-documented in general.  I've
added your comment to the todo list.

> [7]
> I think a wrapper like I've attached would be pretty useful for dealing
> with time values - mainly for testing/debugging.
> 
> (as a matter of fact, this simple wrapper helped me a lot catch a 
> few bugs from code written by others. all I had to do was replace 
> time_t with time_t_wrapper<>)
> 
> Short description:
> - this allows writing times (to streams) in a user-friendly manner:
>   besides the time_t value, a *word* that shows what the time_t 
> value means. - also, reading these values from streams can be done 
> using the '>>' operator.  The cool thing is that both time_t and 
> time_t_wrapper<> values can be read  from a stream.
> 
> time_t_wrapper<> can be used in two modes:
> - debug mode
> - release mode (time_t_wrapper<> can be as efficient as time_t in release
> mode)
> 
> In debug mode, each value contains a user-friendly string 
> corresponding to the time_t value.

The reason that I didn't put any time_t related interfaces into
the library is that I wanted to encourage people away from using
'under-abstracted clock-hardware-limited' time representations 
in code.  I suspect you may have spent many hours, as I have, 
debugging errors related to the use of time_t.  I mean, time_t
is 1/1/1970 0:0:0, right?  Oops, minor point, I mean 
UTC 1/1/1970 0:0:0.  Was that time_t really a time value or
was it a time duration?  Hard to tell without reading (the
hopefully consistent) code...

So, my stance on it has been to encourage people to use the 
library and abandon time_t, period.  With a good compiler
(and perhaps some enhancement) the library should be almost
as efficient as using time_t. And I really haven't been
asked much about time_t support. However, I realize this is not 
viable for some projects with large legacy code bases to just 
convert across the board.  Hence I can see the need for the 
from_time_t function, as a bridge between legacy code.

As for your wrapper, I think you are addressing a more general
topic of layering a debug wrapper around a simple type.  Certainly
can be a useful technique.  That said, if you have the opportunity
to modify the source why not just convert to using the library?
It will give you more streaming options than the wrapper class
without having to write all that code.

> As a side-note, this could be made much more general, to work for other
> HANDLE-like types (for instance, HWND in Win32 or so).
> What do you think?

Sure, I can see these sort of wrappers being useful.  I'm a big
believer in small types that enforce consistency for big programs.

Thanks for the suggestions.

Jeff