Skip Menu |

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

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

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

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



Subject: [Enhancement request] parse_duration() should indicate errors and new() should have an on_error parameter
I find it rather unhelpful that when parse_duration() fails, it returns a DateTime::Duration object without producing any error. If you then make use of that object (and there is no reason why you wouldn't do, given that there was no error!) then subsequent calculations can go wrong because the object represents a duration of 0. Other DateTime::Format::* modules have an on_error parameter in their constructors to give control over the behaviour when parse_datetime() fails (e.g. return the undefined value, croak() or run a callback subroutine), and they also default to one of those behaviours. It would be helpful if DateTime::Format::Duration did likewise. For example, DateTime::Format::Strptime has an on_error parameter which defaults to 'undef', so if parse_datetime() fails then it returns the undefined value by default.
On Tue Nov 10 08:22:21 2009, SHAY wrote: Show quoted text
> I find it rather unhelpful that when parse_duration() fails, it returns > a DateTime::Duration object without producing any error. If you then > make use of that object (and there is no reason why you wouldn't do, > given that there was no error!) then subsequent calculations can go > wrong because the object represents a duration of 0. > > Other DateTime::Format::* modules have an on_error parameter in their > constructors to give control over the behaviour when parse_datetime() > fails (e.g. return the undefined value, croak() or run a callback > subroutine), and they also default to one of those behaviours. It would > be helpful if DateTime::Format::Duration did likewise. > > For example, DateTime::Format::Strptime has an on_error parameter which > defaults to 'undef', so if parse_datetime() fails then it returns the > undefined value by default.
I would also like to see such behaviour added. Attached is a patch which adds an on_error parameter, taking "zero" (current behaviour), "undef", or "croak" (as per other modules).
Subject: DateTime-Format-Duration-on_error.patch
--- Duration.pm.orig 2010-02-05 19:38:39.000000000 +0000 +++ Duration.pm 2010-02-05 20:05:19.000000000 +0000 @@ -26,6 +26,7 @@ base => { type => OBJECT | UNDEF, default => undef }, normalise => { type => SCALAR, default => 0 }, normalize => { type => SCALAR, default => 0 }, + on_error => { type => SCALAR, default => "zero" }, }); $args{normalise} ||= delete $args{normalize}; @@ -193,9 +194,9 @@ sub parse_duration { my $self = shift; - DateTime::Duration->new( - $self->parse_duration_as_deltas(@_) - ); + my @deltas = $self->parse_duration_as_deltas(@_); + + return (defined $deltas[0]) ? DateTime::Duration->new( @deltas ) : undef; } sub parse_duration_as_deltas { @@ -216,9 +217,19 @@ # Run the parser my $parser = $self->{parser} || $self->_build_parser; - eval($parser); + my $retval = eval($parser); die "Parser ($parser) died:$@" if $@; + if (!$retval) { + if ($self->{'on_error'} eq "zero") { + # Nothing + } elsif ($self->{'on_error'} eq "undef") { + return undef; + } elsif ($self->{'on_error'} eq "croak") { + croak "Failed to parse duration from '$time_string'"; + } + } + $years += ($centuries * 100); $days += ($weeks * 7 ); @@ -768,6 +779,12 @@ If a base DateTime is given then that is the normalisation date. Setting this attribute overrides the above option and sets normalise to true. +=item * C<on_error => $string> + +This controls behaviour after failing to parse a string. Can be "zero" +(default) which returns a zero duration, "undef" which returns undef, +or "croak" which croaks. + =back =back