On Fri, Jan 24, 2003 at 05:33:05PM -0600, Dave Rolsky wrote:
> On Fri, 24 Jan 2003, Tim Bunce wrote:
>
> > Would someone do me the great favour of summarizing some things for me?
> >
> > Just the Goals and Design Principles would be great, an outline of
> > the API(s) would be a bonus.
> >
> > I'm particularly interested in:
> > Basic object API (there is going to be an object right?)
>
> Yes, it will be an object. The API is not even close to finalized, but
> you can take a look at the docs for the DateTime.pm module in CVS to get
> an idea of where it's going:
>
>
>http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/perl-date-time/modules/DateTime.pm/lib/DateTime.pm?rev=1.16&content-type=text/vnd.viewcvs-markup
Thanks. Some comments:
I've a bunch of relatively minor efficiency concerns but I'll address
those by sending a patch once more dust has settled :)
The 'floating' param needs a better name. Perhaps 'no_timezone', or
at least someing that better indicates it's related to timezones.
Maybe just use a specific value for the 'time_zone' parameter:
time_zone => undef.
The "elsif ( exists( $args{offset} ) ) " branch at the end of new()
contains "if ($args{floating} " which will never be true there.
All internal functions (not methods) should probably begin with an
underscore for the same reasons as internal methods do.
Class data is bad (just doesn't scale to big applications, consider
mod_perl for example). Use of class data should be limited to
providing default values for undefined object attributes.
The docs for new() are confusing about timezone handling.
First it says "It defaults times to UTC (Greenwich Mean Time, also
called Zulu)." Then "The new() method tries to be intelligent about
figuring out your local time zone. If you enter a time that's not
*explicitly* in UTC, it looks at the environment variable C<TZ>".
The ymd, mdy, and dmy methods all use two digit years. That's not
ideal but I can understand people might like it. I'd strongly suggest
that they be renamed yymmdd, mmddyy, ddmmyy, and the corresponding
four digit year methods added as: yyyymmdd, mmddyyyy, ddmmyyyy.
(Similary hms could be renamed hhmmss for consistency.)
The parsetime method should be renamed hour_min_sec and return
values in that order (removing the need for a reverse in hms)
and documented as a public method. It's very common for all to be
needed together.
A corresponding public year_month_day method should be provided.
So your example:
> The core functionality will be extensible by simply by having a
> well-defined, simple API. For example, a module that provides ICal
> datetime format output does this:
>
> my $date = join '', $dt->year, $dt->month, $dt->day;
> $date .= join '', $dt->hour, $dt->minute, $dt->second;
could now use:
my $date = join '', $dt->year_month_day
$date .= join '', $dt->hour_min_sec;
and save four method calls.
I think hour_min_sec (aka parsetime) either shouldn't int() the seconds
or a separate hour_min_secf method should be provided.
> As for speed, there's a couple ways to attack this. First of all, a lot
> of the calculations the core object will be doing can be memoized quite
> easily, and reset only when the object is changed (which will be
> infrequent in most uses).
Don't memoize until the API is stable and only after you've given myself
and others a chance to optimize the internals. Most of the core
DateTime.pm code can probably be made fast enough that the overhead
of memoizing may be too great to make an improvement.
> The other bottleneck that may occur is the time zone code. Since this
> will largely be generated from the Olson DB, it's not too much of a
> stretch to imagine generating XS code instead of Perl.
Again, don't rush. The most important thing to get right is the API.
All else is changable in time.
Overall it looks good.
Tim.
