houseabsolute / DateTime.pm

A date and time object for Perl
https://metacpan.org/release/DateTime/
Other
47 stars 49 forks source link

String comparison of two DateTime objects too accurate? #30

Open autarch opened 7 years ago

autarch commented 7 years ago

Migrated from rt.cpan.org #57743 (status was 'open')

Requestors:

From mschwern@cpan.org (@schwern) on 2010-05-22 00:31:33:

DateTime currently uses (after a lot of scaffolding) utc_rd_values() to perform a string comparison between two DateTime objects. This takes into account nanoseconds. Since the default string form does not show nanoseconds it leads to the strange case where two apparently equal strings are not equal.

For example:

use DateTime;

my $foo = DateTime->new(year => 2010, nanosecond => 123); my $bar = DateTime->new(year => 2010, nanosecond => 456);

print $foo eq $bar ? "Equal" : "Not equal"; print $foo; print $bar;' END Not equal 2010-01-01T00:00:00 2010-01-01T00:00:00

I'm not sure what's to be done with this, but it is difficult to debug.

One choice is to leave "eq" as a string comparison operator and just compare DateTime objects as strings rather than as dates. This removes the surprise, but it also removes a possibly valuable increased accuracy and its probably backwards compatible.

Another is to compare using lower accuracy, one more in line with what's displayed, such as utc_rd_as_seconds. This will bring the results in line with the level of accuracy shown in the default string representation while retaining cross time zone equality. OTOH people may change the string format to one that includes nanoseconds, and its probably backwards incompatible.

Another is to throw a warning when a string comparison fails because of a nanosecond difference. OTOH this may get noisy.

autarch commented 7 years ago

From autarch@urth.org (@autarch) on 2010-05-22 03:44:20:

On Fri, 21 May 2010, Michael G Schwern via RT wrote:

One choice is to leave "eq" as a string comparison operator and just compare DateTime objects as strings rather than as dates. This removes the surprise, but it also removes a possibly valuable increased accuracy and its probably backwards compatible.

Another is to compare using lower accuracy, one more in line with what's displayed, such as utc_rd_as_seconds. This will bring the results in line with the level of accuracy shown in the default string representation while retaining cross time zone equality. OTOH people may change the string format to one that includes nanoseconds, and its probably backwards incompatible.

Another is to throw a warning when a string comparison fails because of a nanosecond difference. OTOH this may get noisy.

I'm not sure I have much to add. These are all possibilities. I'm not sure that it's worth breaking back compat to try to "fix" this.

Have I mentioned that I really, really, really hate Perl's overloading recently?

-dave

/============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================/

autarch commented 7 years ago

From mrdvt@cpan.org on 2010-06-29 17:20:14:

I think must people expect stringify then equate for the "eq" overload so I don't think it’s breaking anything. If you have a formatter in place, DateTime should use that to stringify as well.

print "$foo" eq "$bar" ? "Equal" : "Not equal";

However, the "==" overload should be down to the nanosec.

Thanks, Mike

mrdvt

autarch commented 7 years ago

From drolsky@cpan.org (@autarch) on 2012-11-16 17:36:30:

On Tue Jun 29 13:20:14 2010, MRDVT wrote:

I think must people expect stringify then equate for the "eq" overload so I don't think it’s breaking anything. If you have a formatter in place, DateTime should use that to stringify as well.

print "$foo" eq "$bar" ? "Equal" : "Not equal";

However, the "==" overload should be down to the nanosec.

I'm looking at this again (I'm slow).

It breaks a test to change this, which means it breaks backwards compat. Changing this also means that two otherwise identical objects with different formatters would not compare as equal.

I think I should've done it your way to start with, but it might be too late to change it.

autarch commented 7 years ago

From mschwern@cpan.org (@schwern) on 2012-11-21 22:09:38:

On Fri Nov 16 12:36:30 2012, DROLSKY wrote:

It breaks a test to change this, which means it breaks backwards compat.

Test or no, I agree this is a clear change to existing, documented functionality. It comes down to which is worse: continuing the current surprising and difficult to debug behavior into the future, or a one time break.

Changing this also means that two otherwise identical objects with different formatters would not compare as equal.

That's ok, we're comparing them as strings, it should compare the strings. You get what you asked for. Changing the behavior of overloaded operators is the root of all evil.

compare() exists for exact date equality. Leave == unimplemented. DateTime has no numeric value, it shouldn't have a == operator. Bending the meaning of operators is what got things into this state in the first place.

One thing which the current implementation of eq does well is it allows comparison of DateTime objects and strings while allowing DateTime vs DateTime comparisons to work with high accuracy and regardless of format.

One flaw is this only works if all the DateTime objects have a formatter which matches the format of the strings. Since the formatter is attached to the object, not the comparison, this can be difficult to guarantee. So perhaps a better way to handle this is to supply a class method which does the comparison.

my $cmp = DateTime->compare_with_strings($a, $b, $formatter_class);

If $a and $b are DateTime objects, it acts like compare(). If neither $a nor $b are DateTime objects, it does an eq. If only one of $a or $b is a DateTime object it first runs it through the given $formatter_class and then does an eq on the strings. This guarantees that a a list of formatted date strings and arbitrary DateTime objects will compare properly.