pugs-comm...@feather.perl6.nl wrote:
Author: wayland
Date: 2009-02-19 08:47:26 +0100 (Thu, 19 Feb 2009)
New Revision: 25405

Modified:
   docs/Perl6/Spec/S32-setting-library/Temporal.pod
Log:
Improved Temporal (previously DateTime) stuff a bit

Okay, so I have a few specific suggestions ...

And by the way, this is easier to read at http://perlcabal.org/svn/pugs/view/docs/Perl6/Spec/S32-setting-library/Temporal.pod where you can just see the current version ...

+role   Temporal::Date {
+       has Int $.year;
+       has Int $.month;
+       has Int $.day; # Day of month
+       has Int $.dayofweek;

You could make month/day into positive integers or "subtype of Int where 1..12" etc, though it isn't strictly necessary. Leave year as Int of course, since at least in the proleptic Gregorian etc you can have negatives or zero.

+       has Int $.era; # BC, AD, etc, depending on locale
+       has Str $.defaultformat; # A CLDR-formatted string, for use with 
toString();

Now $.era you definitely should change, either to Str like defaultformat or come up with some enumerated type, as Int seems wrong there, though Str is probably best to make later extensions easier, especially if you're sticking with plain Int for the others.

+       multi method infix:{'<=>'}(Temporal::Date $self, Temporal::Duration 
$other);

I don't think it makes sense to compare any kind of Instant/Date/Time to a Duration for relative order; they are not the same thing like a position isn't the same as a length.

+role   Temporal::Time {
        has $.hour;
        has $.minute;
        has $.second;

You didn't give these attributes types like you did for Date.

I suggest Int (or UInt) for hour/minute if you want to be consistent, and Rat/Num for second.

+       multi method infix:{'<=>'}(Temporal::Time $self, Temporal::Duration 
$other);

Likewise with the Date example this doesn't make sense, comparing a position to a length.

+role   Temporal::Subsecond {
+       has     $.nanosecond;
+}

Two things:

1. You should just make that a Rat/Num $second or $frac_second or something. I know DateTime.pm has the precedent but counting in nanoseconds is too arbitrary, especially since newer/future machines would have a higher resolution than that. Using Rat/Num will let this be open-ended.

2. Why not just make normal $.second in Time be a Rat/Num instead? Users can always subtype it with "where is_desired_precision()" or something if they want that. Separating out whole and fractional seconds has a design smell to me since they measure the same unit (unlike min/hour/day/mon/year, each of which deserve to be separate).


+class Temporal::Instant
<snip>
+       multi method Temporal::Instant infix:<+>(Temporal::Instant $self, 
Duration $other);
+ multi method infix:<->(Temporal::Instant $self, Duration $other);
+       multi method infix:<->(Temporal::Instant $self, Duration $other);

You repeated '-' and also the declaration is different than that for '+'; I assume you wanted to make your '-' like your '+'.

+       multi method infix:{'<=>'}(Temporal::Instant $self, Duration $other);

Again with the doesn't make sense as per Duration <=> Date|Time.

+class Temporal::Duration + does Temporal::Date + does Temporal::Time + does Temporal::Subsecond
+{
+}

Okay, I see why you did what you did with those 3 above "doesn't make sense" examples; but in that case, I think what you really want is to move the "<=>" out of Date|Time and into Duration. And stuff.

In fact, while they look similar, I wouldn't reuse the Date|Time roles between Instant and Duration. While they consist of the same units, the allowed values differ; for Duration any Int is allowed while for Instant generally only non-negative constrained values like 1..23 etc are allowed.

But who knows, maybe you can still make the idea of sharing roles in that way work, and I do see it making sense in some ways.

+=head2 Temporal::Recurring
+
This class specifies when a repetitive action (eg. a cron job) happens. -class DateTime::Recurring {
+class  Temporal::Recurring {
 ...
 }

I should point out that in some respects any Time is effectively a Recurring because it doesn't specify a day and so could mean "on every day"; similarly eg if you have a Date but leave the Year undefined (repeat every year) and so on. Unless your Instant|Date|Time requires all attributes to be defined, or all unit defined attributes than any defined attribute to be defined.

Or more likely in practice, leaving those parts out just means "don't know" and not recurring.

But a dedicated Recurring is definitely useful since it can specify other kinds of recurrings, like "every 90 minutes from X Instant" for example, and it unambiguously means recurring rather than don't know.

-- Darren Duncan

Reply via email to