Hi! On Wed, Jun 27, 2012 at 07:16:06AM +0100, Zefram wrote: > Thomas Klausner wrote: > >The only potential deal-breaker is the fact that if you now create a new > >DateTime that does not exists, you'll get the next valid time instead of > >an exception > > Getting an exception in this situation is a feature, not a bug.
I do not agree, but more from a pragmatic point of view. I understand the point of view that non-existent DateTimes should die, but those special cases (Sao Paulo et.al.) are IMO special cases that should be handled one way or the other. Currently, it makes using DateTime very annoying, because we have to code around EVERY DateTime interaction. BTW, if DateTime could handle plain Dates, the problem would also go way. I do not want to rub salt into Daves wounds (http://www.houseabsolute.com/presentations/dates-times-perl-and-you/#28) but a plain DateTime::Date class would have indeed been a good idea. Anyway, back to the hopefully constructive work: > Finding > the next valid local time is potentially useful behaviour, but it needs > to be invoked explicitly, perhaps by an extra constructor parameter. I though a lot about how this could be done. The problem here of course is that DateTime::TimeZone->new is called in various places in DateTime (and potentially other code), so this new parameter would need to be handled by DateTime AND DateTime::TimeZone: DateTime->new( year => 2012, month => 10, day => 21, time_zone=>'America/Sao_Paulo', fix_invalid_local_time => 1 # better name for param is welcome ); Alternativly, we could just somehow enhance the current time_zone parameter: DateTime->new( year => 2012, month => 10, day => 21, time_zone=>'America/Sao_Paulo:FIX', # ugly! ); or probably better, use another data type DateTime->new( year => 2012, month => 10, day => 21, time_zone=>['America/Sao_Paulo' => 'fix_invalid_local_time' ], ); Both of those ideas only have to be implemented in one place: DateTime::TimeZone->new and should neither cause backwards compatibility errors nor be hard to add to existing code by users who like that feature. Any feedback on this suggestions? Any other ideas? > >Oh, and requesting a specific non-existed time also has strange results: > >~$ perl -MDateTime -E 'say > >DateTime->new(year=>2012,month=>10,day=>21,hour=>0,minute=>30,time_zone=>"America/Sao_Paulo")' > >2012-10-21T01:30:00 > > If you're going to advertise this feature as "find the next valid local > time", you need to handle cases like this correctly. This is fixed/implemented by the attached patches. Greetings, domm -- #!/usr/bin/perl http://domm.plix.at for(ref bless{},just'another'perl'hacker){s-:+-$"-g&&print$_.$/}
>From f91bd008dba06bf49a4fa62b34922dae86e37c43 Mon Sep 17 00:00:00 2001 From: Thomas Klausner <[email protected]> Date: Wed, 27 Jun 2012 02:43:54 +0200 Subject: [PATCH 1/2] try to work around nonexistent DST datetimes (incl test) --- lib/DateTime/TimeZone.pm | 8 +++++ t/22_dst_mess.t | 73 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 t/22_dst_mess.t diff --git a/lib/DateTime/TimeZone.pm b/lib/DateTime/TimeZone.pm index 3353188..51111ec 100644 --- a/lib/DateTime/TimeZone.pm +++ b/lib/DateTime/TimeZone.pm @@ -217,6 +217,10 @@ sub _spans_binary_search { my $current = $self->{spans}[$i]; if ( $seconds < $current->[$start] ) { + if ($seconds > $self->{spans}[$i-1][$end]) { + return $self->{spans}[$i-1]; + } + $max = $i; my $c = int( ( $i - $min ) / 2 ); $c ||= 1; @@ -227,6 +231,10 @@ sub _spans_binary_search { } elsif ( $seconds >= $current->[$end] ) { $min = $i; + if ($seconds < $self->{spans}[$i+1][$start]) { + return $current; + } + my $c = int( ( $max - $i ) / 2 ); $c ||= 1; diff --git a/t/22_dst_mess.t b/t/22_dst_mess.t new file mode 100644 index 0000000..a3b7263 --- /dev/null +++ b/t/22_dst_mess.t @@ -0,0 +1,73 @@ +use strict; +use warnings; + +use Test::More; + +use DateTime; + +{ # adding to DateTime skips non-existing times America/Sao_Paulo + my $dt = DateTime->new( + year => 2012, month => 10, day => 20, hour=>23, minute=>59, + time_zone => 'America/Sao_Paulo', + ); + $dt->add(minutes=>1); + is($dt->iso8601,'2012-10-21T01:00:00','America/Sao_Paulo: 2012-10-20T23:59:00 +1min = 2012-10-21T01:00:00'); +} + +{ # adding to DateTime skips non-existing times America/Sao_Paulo + my $dt = DateTime->new( + year => 2012, month => 10, day => 20, + time_zone => 'America/Sao_Paulo', + ); + $dt->add(days=>1); + is($dt->iso8601,'2012-10-21T01:00:00','America/Sao_Paulo: 2012-10-20T23:59:00 +1min = 2012-10-21T01:00:00'); +} + +{ # adding to DateTime skips non-existing times Africa/Cairo + my $dt = DateTime->new( + year => 2010, month => 4, day => 29, hour=>23, minute=>59, + time_zone => 'Africa/Cairo', + ); + $dt->add(minutes=>1); + is($dt->iso8601,'2010-04-30T01:00:00','Africa/Cairo: 2010-04-20T23:59:00 +1min = 2010-04-30T01:00:00'); + +} + +{ # creating new DateTime + my $dt = DateTime->new( + year => 2012, month => 10, day => 21, + time_zone => 'America/Sao_Paulo', + ); + is($dt->iso8601,'2012-10-21T01:00:00','America/Sao_Paulo: new 2012-10-21 -> 2012-10-21T01:00:00'); + +} + +{ # subtracting to DateTime skips non-existing times America/Sao_Paulo + my $dt = DateTime->new( + year => 2012, month => 10, day => 21, hour=>1, minute=>0, + time_zone => 'America/Sao_Paulo', + ); + $dt->subtract(minutes=>1); + is($dt->iso8601,'2012-10-20T23:59:00','America/Sao_Paulo: 2012-10-21T01:00:00 -1min = 2012-10-20T23:59:00'); +} + +{ # truncate + my $dt = DateTime->new( + year => 2012, month => 10, day => 21, hour=>12, minute=>42, + time_zone => 'America/Sao_Paulo', + ); + $dt->truncate(to=>'day'); + is($dt->iso8601,'2012-10-21T01:00:00','America/Sao_Paulo: truncate day 2012-10-21T01:00:00'); +} + +{ # creating new DateTime + my $dt = DateTime->new( + year => 2012, month => 10, day => 21,hour=>0,minute=>30, + time_zone => 'America/Sao_Paulo', + ); + is($dt->iso8601,'2012-10-21T01:30:00','America/Sao_Paulo: new 2012-10-21T00:30:00 -> 2012-10-21T01:30:00'); + +} + + +done_testing(); -- 1.7.10 >From cb94f05d2f536301bae74ffe82c96338c17218f8 Mon Sep 17 00:00:00 2001 From: Thomas Klausner <[email protected]> Date: Sun, 1 Jul 2012 22:50:24 +0200 Subject: [PATCH 2/2] new in invalid time return next valid time --- lib/DateTime/TimeZone.pm | 8 +++++++- t/22_dst_mess.t | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/DateTime/TimeZone.pm b/lib/DateTime/TimeZone.pm index 51111ec..22de3c2 100644 --- a/lib/DateTime/TimeZone.pm +++ b/lib/DateTime/TimeZone.pm @@ -218,7 +218,13 @@ sub _spans_binary_search { if ( $seconds < $current->[$start] ) { if ($seconds > $self->{spans}[$i-1][$end]) { - return $self->{spans}[$i-1]; + my $diff = $self->{spans}[$i-1][$end] - $seconds; + my @copy; + foreach my $val (@{$self->{spans}[$i-1]}) { + push(@copy,$val); + } + $copy[OFFSET] = $copy[OFFSET] - $diff; + return \@copy; } $max = $i; diff --git a/t/22_dst_mess.t b/t/22_dst_mess.t index a3b7263..4c950bf 100644 --- a/t/22_dst_mess.t +++ b/t/22_dst_mess.t @@ -65,7 +65,7 @@ use DateTime; year => 2012, month => 10, day => 21,hour=>0,minute=>30, time_zone => 'America/Sao_Paulo', ); - is($dt->iso8601,'2012-10-21T01:30:00','America/Sao_Paulo: new 2012-10-21T00:30:00 -> 2012-10-21T01:30:00'); + is($dt->iso8601,'2012-10-21T01:00:00','America/Sao_Paulo: new 2012-10-21T00:30:00 -> 2012-10-21T01:00:00'); } -- 1.7.10
