This patch started out as just adding some tests (taken from DT::TZ::Alias) but I
uncovered several issues.
These currently accepted offset formats should be rejected as they are ambiguous:
'111', '+111', '-111',
'1:11', '+1:11', '-1:11',
'11111', '+11111', '-11111',
'111:11', '+111:11', '-111:11',
'1:1111', '+1:1111', '-1:1111',
'1:11:11', '+1:11:11', '-1:11:11',
'1111:11', '+1111:11', '-1111:11',
'11:1111', '+11:1111', '-11:1111',
- the hours value of an offset must be specified as 2 digits
- the colon separator must be used between both hours/minutes and minutes/seconds or
not at all
The algorithm for turning an offset string into a number of seconds was taking a
modulus of the hours value. This could lead to developer surprise. The hours value
has been limited to 99 (what fits into 2 digits), minutes limited to 59, and seconds
limited to 59.
- offset string formats are now validated and undef is returned for bad formats
- the modulus of the hours value is removed
The algorithm for turning a number of seconds into an offset string was producing
floating point values. These were not being seen because of the sprintf format but
could be a problem in the future.
- internal floating point values are now truncated
- the input number of seconds is now limited to what can actually be formated into a
valid offset string
- the output of offset_as_seconds() passed to offset_as_string() will return the
original input (and vice versa)
The 'name' key of OffsetOnly objects was not being normalized. Two objects with
identical values but different 'names' could be created.
- the 'name' value is now normalized
Also:
- added more regression tests
- some minor cosmetic changes for clarity
Comments?
-J
--
Index: lib/DateTime/TimeZone.pm
===================================================================
RCS file: /cvsroot/perl-date-time/modules/DateTime-TimeZone/lib/DateTime/TimeZone.pm,v
retrieving revision 1.84
diff -u -r1.84 TimeZone.pm
--- lib/DateTime/TimeZone.pm 10 Aug 2003 13:44:48 -0000 1.84
+++ lib/DateTime/TimeZone.pm 19 Aug 2003 10:28:53 -0000
@@ -346,12 +346,16 @@
return 0 if $offset eq '0';
- return undef unless $offset =~ /^([\+\-])?(\d\d?):?(\d\d)(?::?(\d\d))?$/;
+ return undef unless $offset =~ /^([\+\-])?(\d\d)(:?)(\d\d)(?:\3(\d\d))?$/;
+
+ my ( $sign, $hours, $minutes, $seconds ) = ( $1, $2, $4, $5 );
- my ( $sign, $hours, $minutes, $seconds ) = ( $1, $2, $3, $4 );
$sign = '+' unless defined $sign;
+ return undef unless $hours >= 0 && $hours <= 99;
+ return undef unless $minutes >= 0 && $minutes <= 59;
+ return undef unless ! defined( $seconds ) || ( $seconds >= 0 && $seconds <= 59 );
- my $total = ($hours * 60 * 60) + ($minutes * 60);
+ my $total = $hours * 3600 + $minutes * 60;
$total += $seconds if $seconds;
$total *= -1 if $sign eq '-';
@@ -363,17 +367,17 @@
my $offset = shift;
return undef unless defined $offset;
+ return undef unless $offset >= -359999 && $offset <= 359999;
my $sign = $offset < 0 ? '-' : '+';
$offset = abs($offset);
- my $hours = $offset / ( 60 * 60 );
- $hours = $hours % 24;
-
- my $mins = ( $offset % ( 60 * 60 ) ) / 60;
-
- my $secs = $offset % 60;
+ my $hours = int( $offset / 3600 );
+ $offset %= 3600;
+ my $mins = int( $offset / 60 );
+ $offset %= 60;
+ my $secs = int( $offset );
return ( $secs ?
sprintf( '%s%02d%02d%02d', $sign, $hours, $mins, $secs ) :
@@ -569,10 +573,12 @@
Given an offset as a string, this returns the number of seconds
represented by the offset as a positive or negative number.
+Returns C<undef> if $offset is not in the range C<-99:59:59> to C<+99:59:59>.
=item * offset_as_string( $offset )
Given an offset as a number, this returns the offset as a string.
+Returns C<undef> if $offset is not in the range C<-359999> to C<359999>.
=back
Index: lib/DateTime/TimeZone/OffsetOnly.pm
===================================================================
RCS file:
/cvsroot/perl-date-time/modules/DateTime-TimeZone/lib/DateTime/TimeZone/OffsetOnly.pm,v
retrieving revision 1.18
diff -u -r1.18 OffsetOnly.pm
--- lib/DateTime/TimeZone/OffsetOnly.pm 13 Jun 2003 17:50:57 -0000 1.18
+++ lib/DateTime/TimeZone/OffsetOnly.pm 19 Aug 2003 10:28:53 -0000
@@ -3,7 +3,7 @@
use strict;
use vars qw ($VERSION);
-$VERSION = 0.01;
+$VERSION = 0.02;
use DateTime::TimeZone;
use base 'DateTime::TimeZone';
@@ -24,7 +24,10 @@
return DateTime::TimeZone::UTC->new unless $offset;
- my $self = { name => $p{offset}, offset => $offset };
+ my $self = {
+ name => DateTime::TimeZone::offset_as_string( $offset ),
+ offset => $offset,
+ };
return bless $self, $class;
}
Index: t/05offset.t
===================================================================
RCS file: /cvsroot/perl-date-time/modules/DateTime-TimeZone/t/05offset.t,v
retrieving revision 1.2
diff -u -r1.2 05offset.t
--- t/05offset.t 7 May 2003 17:08:14 -0000 1.2
+++ t/05offset.t 19 Aug 2003 10:28:53 -0000
@@ -4,7 +4,7 @@
use DateTime::TimeZone;
-use Test::More tests => 9;
+use Test::More tests => 28;
is(DateTime::TimeZone::offset_as_string(0), "+0000",
"offset_as_string does the right thing on 0");
@@ -16,14 +16,54 @@
"offset_as_string works on positive half hours");
is(DateTime::TimeZone::offset_as_string(-5400), "-0130",
"offset_as_string works on negative half hours");
-
is(DateTime::TimeZone::offset_as_string(20700), "+0545",
"offset_as_string works on positive 15min zones");
is(DateTime::TimeZone::offset_as_string(-20700), "-0545",
"offset_as_string works on negative 15min zones");
+is(DateTime::TimeZone::offset_as_string(359999), "+995959",
+ "offset_as_string max value");
+is(DateTime::TimeZone::offset_as_string(-359999), "-995959",
+ "offset_as_string min value");
+is(DateTime::TimeZone::offset_as_string(360000), undef,
+ "offset_as_string exceeded max value");
+is(DateTime::TimeZone::offset_as_string(-360000), undef,
+ "offset_as_string exceeded min value");
+
+my @offset_seconds = qw(
+ 0
+ 3600
+ -3600
+ 5400
+ -5400
+ 20700
+ -20700
+ 359999
+ -359999
+);
+
+my @offset_strings = qw(
+ +0100
+ -0100
+ +0130
+ -0130
+ +0545
+ -0545
+ +995959
+ -995959
+);
-is(DateTime::TimeZone::offset_as_string(86400), "+0000",
- "offset_as_string rolls over properly on one full day of seconds");
-is(DateTime::TimeZone::offset_as_string(86400 + 3600), "+0100",
- "offset_as_string rolls over properly on one day + 1 hour of seconds");
+foreach ( @offset_seconds ) {
+ is( DateTime::TimeZone::offset_as_seconds(
+ DateTime::TimeZone::offset_as_string( $_ )
+ ),
+ $_, "n -> offset_as_string -> offset_as_seconds = n "
+ );
+}
+foreach ( @offset_strings ) {
+ is( DateTime::TimeZone::offset_as_string(
+ DateTime::TimeZone::offset_as_seconds( $_ )
+ ),
+ $_, "n -> offset_as_seconds -> offset_as_string= n "
+ );
+}
Index: t/07offset-only.t
===================================================================
RCS file: /cvsroot/perl-date-time/modules/DateTime-TimeZone/t/07offset-only.t,v
retrieving revision 1.3
diff -u -r1.3 07offset-only.t
--- t/07offset-only.t 7 May 2003 17:08:14 -0000 1.3
+++ t/07offset-only.t 19 Aug 2003 10:28:53 -0000
@@ -9,7 +9,7 @@
BEGIN { require 'check_datetime_version.pl' }
-plan tests => 2;
+plan tests => 180;
eval { DateTime::TimeZone::OffsetOnly->new( offset => 'bad' ) };
is( $@, "Invalid offset: bad\n",
@@ -17,3 +17,92 @@
my $off = DateTime::TimeZone::OffsetOnly->new( offset => '-0100' );
is( $off->name, '-0100', 'name is -0100' );
+
+my @good_offsets = (
+ [ '0', 'UTC' ],
+ [ '0000', 'UTC' ],
+ [ '000000', 'UTC' ],
+ [ '1000', '+1000' ],
+ [ '100001', '+100001' ],
+ [ '10:00:02', '+100002' ],
+ [ '+0000', 'UTC' ],
+ [ '+000000', 'UTC' ],
+ [ '+000001', '+000001' ],
+ [ '+00:00:02', '+000002' ],
+ [ '-0000', 'UTC' ],
+ [ '-000000', 'UTC' ],
+ [ '-000001', '-000001' ],
+ [ '-00:00:02', '-000002' ],
+ [ '+9959', '+9959' ],
+ [ '+995959', '+995959' ],
+ [ '+99:59:58', '+995958' ],
+ [ '-9959', '-9959' ],
+ [ '-995959', '-995959' ],
+ [ '-99:59:58', '-995958' ],
+);
+
+my @bad_offsets = (
+ '+0', '-0',
+ '1', '+1', '-1',
+ '1:', '+1:', '-1:',
+ ':1', '+:1', '-:1',
+ '11', '+11', '-11',
+ '11:', '+11:', '-11:',
+ '1:1', '+1:1', '-1:1',
+ ':11', '+:11', '-:11',
+ '111', '+111', '-111',
+ '111:', '+111:', '-111:',
+ '11:1', '+11:1', '-11:1',
+ '1:11', '+1:11', '-1:11',
+ ':111', '+:111', '-:111',
+ ':11:1', '+:11:1', '-:11:1',
+ '1:11:', '+1:11:', '-1:11:',
+ '1111:', '+1111:', '-1111:',
+ '111:1', '+111:1', '-111:1',
+ '1:111', '+1:111', '-1:111',
+ ':1111', '+:1111', '-:1111',
+ ':11:11', '+:11:11', '-:11:11',
+ '1:11:1', '+1:11:1', '-1:11:1',
+ '11:11:', '+11:11:', '-11:11:',
+ '11111', '+11111', '-11111',
+ '11111:', '+11111:', '-11111:',
+ '1111:1', '+1111:1', '-1111:1',
+ '111:11', '+111:11', '-111:11',
+ '11:111', '+11:111', '-11:111',
+ '1:1111', '+1:1111', '-1:1111',
+ ':11111', '+:11111', '-:11111',
+ ':11:111', '+:11:111', '-:11:111',
+ '1:11:11', '+1:11:11', '-1:11:11',
+ '11:11:1', '+11:11:1', '-11:11:1',
+ '111:11:', '+111:11:', '-111:11:',
+ '111111:', '+111111:', '-111111:',
+ '11111:1', '+11111:1', '-11111:1',
+ '1111:11', '+1111:11', '-1111:11',
+ '111:111', '+111:111', '-111:111',
+ '11:1111', '+11:1111', '-11:1111',
+ '1:11111', '+1:11111', '-1:11111',
+ ':111111', '+:111111', '-:111111',
+ ':11:1111', '+:11:1111', '-:11:1111',
+ '1:11:111', '+1:11:111', '-1:11:111',
+ '111:11:1', '+111:11:1', '-111:11:1',
+ '1111:11:', '+1111:11:', '-1111:11:',
+ '1111111', '+1111111', '-1111111',
+ '00:60', '+00:60', '-00:60',
+ '00:99', '+00:99', '-00:99',
+ '00:60:00', '+00:60:00', '-00:60:00',
+ '00:99:00', '+00:99:00', '-00:99:00',
+ '00:00:60', '+00:00:60', '-00:00:60',
+ '00:00:99', '+00:00:99', '-00:00:99',
+ '00:60:60', '+00:60:60', '-00:60:60',
+ '00:99:99', '+00:99:99', '-00:99:99',
+);
+
+foreach ( @good_offsets ) {
+ my $off = DateTime::TimeZone::OffsetOnly->new( offset => $_->[0] );
+ is( $off->name, $_->[1] );
+}
+
+foreach ( @bad_offsets ) {
+ eval{ DateTime::TimeZone::OffsetOnly->new( offset => $_ ) };
+ like( $@, qr/Invalid offset/ );
+}
