| Subject: | set_formatter is not validated and doesn't return the datetime object |
Hi,
The set_formatter method does not validate the provided value, and does
not return the DateTime object that is being set. This breaks from the
advertised convention of pre-emptive validation and chainable methods.
I've attached a patch for DateTime 0.55 that includes a fix and tests
for both of these issues.
Regards,
Andrew Whatson
| Subject: | datetime_set_formatter_fix.diff |
=== modified file 'lib/DateTime.pm'
--- lib/DateTime.pm 2010-06-18 03:48:10 +0000
+++ lib/DateTime.pm 2010-06-18 04:12:06 +0000
@@ -172,6 +172,8 @@
type => SCALAR | OBJECT,
optional => 1
},
+ formatter =>
+ { type => SCALAR | OBJECT, can => 'format_datetime', optional => 1 },
};
my $NewValidate = {
@@ -180,8 +182,6 @@
type => SCALAR | OBJECT,
default => 'floating'
},
- formatter =>
- { type => SCALAR | OBJECT, can => 'format_datetime', optional => 1 },
};
sub new {
@@ -1883,10 +1883,8 @@
sub set_minute { $_[0]->set( minute => $_[1] ) }
sub set_second { $_[0]->set( second => $_[1] ) }
sub set_nanosecond { $_[0]->set( nanosecond => $_[1] ) }
-
-sub set_locale { $_[0]->set( locale => $_[1] ) }
-
-sub set_formatter { $_[0]->{formatter} = $_[1] }
+sub set_locale { $_[0]->set( locale => $_[1] ) }
+sub set_formatter { $_[0]->set( formatter => $_[1] ) }
{
my %TruncateDefault = (
=== added file 't/44set_formatter.t'
--- t/44set_formatter.t 1970-01-01 00:00:00 +0000
+++ t/44set_formatter.t 2010-06-18 05:17:23 +0000
@@ -0,0 +1,31 @@
+use strict;
+use warnings;
+
+use Test::Exception;
+use Test::More;
+
+use DateTime;
+use overload;
+
+my $dt = DateTime->now;
+
+throws_ok { $dt->set_formatter('Invalid::Formatter') }
+qr/does not have the method: 'format_datetime'/, 'set_format is validated';
+
+SKIP:
+{
+ skip 'This test requires DateTime::Format::Strptime', 1
+ unless eval 'use DateTime::Format::Strptime; 1';
+
+ my $formatter = DateTime::Format::Strptime->new(
+ pattern => '%Y%m%d %T',
+ );
+
+ is(
+ overload::StrVal($dt->set_formatter($formatter)),
+ overload::StrVal($dt),
+ 'set_format returns the datetime object'
+ );
+}
+
+done_testing();