Skip Menu |

This queue is for tickets about the DateTime-Format-Duration CPAN distribution.

Report information
The Basics
Id: 25078
Status: open
Priority: 0/
Queue: DateTime-Format-Duration

People
Owner: Nobody in particular
Requestors: TWILDE [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 1.02
Fixed in: (no value)



Subject: Odd malfunction of normalize()
The normalize subroutine is inexplicably breaking with a very particular pair of dates. My relevant code looks like: sub timeconv { my($expiry, $block_time) = @_; my $duration = $expiry - $block_time; use Data::Dumper; print Dumper \$block_time, \$expiry, \$duration; my $formatter = DateTime::Format::Duration->new( pattern => '%Y years, %m months, %e days, %H hours, %M minutes, %S seconds', normalize => 1, base => $block_time, ); my %normalized = $formatter->normalize($duration); print Dumper \%normalized; my @periods = ('years','months','days','hours','minutes','seconds'); my $output; if ($normalized{'minutes'} || $normalized{'seconds'}) { $output = sprintf('until %s %s ', $expiry->ymd, $expiry->hms); } else { foreach (@periods) { $output .= sprintf('%s %s, ', $normalized{$_}, $_) if $normalized{$_}; if ($normalized{$_} == 1) { my $singular = $_; $singular =~ s/s$//; $output =~ s/$_/$singular/; } } $output =~ s/, $/ /; } return $output; } $expiry and $block_time are DateTime objects, in UTC. For most cases, this is working fine. In one particular case, though, it breaks down. The output of the first Data::Dumper call in this situation looks like: $VAR1 = \bless( { 'local_rd_secs' => 5297, 'local_rd_days' => 732729, 'rd_nanosecs' => 0, 'locale' => bless( { 'default_time_format_length' => 'medium', 'native_territory' => 'United States', 'native_language' => 'English', 'real_class' => 'en', 'native_complete_name' => 'English United States', 'en_language' => 'English', 'default_date_format_length' => 'medium', 'id' => 'en_US', 'en_territory' => 'United States', 'en_complete_name' => 'English United States' }, 'DateTime::Locale::en' ), 'local_c' => { 'hour' => 1, 'second' => 17, 'month' => 2, 'quarter' => 1, 'day_of_year' => 53, 'day_of_quarter' => 53, 'minute' => 28, 'day' => 22, 'day_of_week' => 4, 'year' => 2007 }, 'utc_rd_secs' => 5297, 'formatter' => undef, 'tz' => bless( { 'name' => 'UTC' }, 'DateTime::TimeZone::UTC' ), 'utc_year' => 2008, 'utc_rd_days' => 732729, 'offset_modifier' => 0 }, 'DateTime' ); $VAR2 = \bless( { 'local_rd_secs' => 5297, 'local_rd_days' => 732771, 'rd_nanosecs' => 0, 'locale' => ${$VAR1}->{'locale'}, 'local_c' => { 'hour' => 1, 'second' => 17, 'month' => 4, 'quarter' => 2, 'day_of_year' => 95, 'day_of_quarter' => 5, 'minute' => 28, 'day' => 5, 'day_of_week' => 4, 'year' => 2007 }, 'utc_rd_secs' => 5297, 'formatter' => undef, 'tz' => bless( { 'name' => 'UTC' }, 'DateTime::TimeZone::UTC' ), 'utc_year' => 2008, 'utc_rd_days' => 732771, 'offset_modifier' => 0 }, 'DateTime' ); $VAR3 = \bless( { 'seconds' => 0, 'minutes' => 0, 'end_of_month' => 'wrap', 'nanoseconds' => 0, 'days' => 11, 'months' => 1 }, 'DateTime::Duration' ); While the second outputs: $VAR1 = { 'seconds' => 0, 'hours' => 0, 'years' => 0, 'minutes' => 0, 'months' => 1, 'days' => 8, 'nanoseconds' => 0 }; And the subroutine itself, using %normalized, outputs "1 month 8 days". Something is obviously going wrong, causing DateTime::Format::Duration to normalize 1 month 11 days to 1 month 8 days, though I can't figure out what. Ideas?
Aha. The problem here is the order in which the math is done. Even though I'm passing a DateTime::Duration object to normalize(), the deltas in that object are being re-added to the DateTime object to come up with the deltas actually used. But, when re-adding deltas like this, order can have an impact. Because the 1 month in the duration object is added first, and February is a 28 day month, boom, we lose 3 days. Output with $formatter->{'diagnostic'} = 1 shows this: Pre Normalise: $VAR1 = { 'seconds' => 0, 'minutes' => 0, 'nanoseconds' => 0, 'days' => 11, 'months' => 1 }; Adding years: 2007-02-22T01:28:17 Adding 1 months: 2007-03-22T01:28:17 Adding 11 days: 2007-04-02T01:28:17 Adding hours: 2007-04-02T01:28:17 Adding 0 minutes: 2007-04-02T01:28:17 Adding 0 seconds: 2007-04-02T01:28:17 Adding 0 nanoseconds: 2007-04-02T01:28:17 Post DateTime::Duration: $VAR1 = { 'seconds' => 0, 'minutes' => 0, 'nanoseconds' => 0, 'days' => 8, 'months' => 1 }; Adding the days first, then the months, would do the trick in this case, though I don't know if it would work in the general case. Actually, the best fix would probably be to entirely reverse the order in which additions are done, starting from nanoseconds and working up to years.
Per the documentation for DateTime, addition and subtraction is always done in this order: days, months, minutes, seconds, nanoseconds It is unclear how this translates out to years and hours. My suspicion is that they are internally converted into months and minutes, respectively. I don't see any downside to simply reversing the order in which delta items are passed to ->add() in the foreach loop: # Can't just add the hash as ->add(%delta) because of mixed positivity: foreach (qw/years months days hours minutes seconds nanoseconds/) { $end->add( $_ => $delta{$_}||0 ); print "Adding $delta{$_} $_: " . $end->datetime . "\n" if $self->{diagnostic}; } Becomes: # Can't just add the hash as ->add(%delta) because of mixed positivity: foreach (qw/nanoseconds seconds minutes hours days months years/) { $end->add( $_ => $delta{$_}||0 ); print "Adding $delta{$_} $_: " . $end->datetime . "\n" if $self->{diagnostic}; }
Last time I'll reply - I've tested this change with all of my major test cases for my application, and it appears to do the right thing in all of them, including the one that broke originally. I've attached a one-line patch.
--- Duration.pm.orig Wed Feb 21 21:42:04 2007 +++ Duration.pm Wed Feb 21 21:42:35 2007 @@ -263,7 +263,7 @@ my $start = $self->base; my $end = $self->base->clone; # Can't just add the hash as ->add(%delta) because of mixed positivity: - foreach (qw/years months days hours minutes seconds nanoseconds/) { + foreach (qw/nanoseconds seconds minutes hours days months years/) { $end->add( $_ => $delta{$_}||0 ); print "Adding $delta{$_} $_: " . $end->datetime . "\n" if $self->{diagnostic}; }