Skip Menu |

This queue is for tickets about the Number-Fraction CPAN distribution.

Report information
The Basics
Id: 17922
Status: rejected
Priority: 0/
Queue: Number-Fraction

People
Owner: DAVECROSS [...] cpan.org
Requestors: davidrw [...] cpan.org
Cc:
AdminCc:

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



Subject: [patch] comparison operator overloads
This is a patch for Number-Fraction-1.08 to add in the 'cmp' and '<=>' (and thus through magic autogeneration all of lt, gt, le, ge, eq, ne and <, >, <=, >=, ==, != ) operators for Number::Fraction objects. Number-Fraction-compare.txt contains a compare() sub w/pod and the modified 'use overload' statement. Note: i made <=> and cmp be the same and do numeric comparison .. if that's undesirable, compare() can be copied to string_compare() and all the ->to_num() calls changed to ->to_string() calls. The second file comparison.t has a bunch of tests (and examples) to verify that it's all working .. --david
Subject: Number-Fraction-compare.txt
Download Number-Fraction-compare.txt
application/force-download 818b

Message body not shown because it is not plain text.

Subject: comparison.t
#!/usr/bin/perl use strict; use warnings; use Test::More tests => 47; use Number::Fraction; my $x = Number::Fraction->new('1/2'); my $y = Number::Fraction->new('1/4'); my $z = Number::Fraction->new('1/33'); is( $x == '1/2', 1, "x == '1/2'" ); is( $y == '1/4', 1, "y == '1/4'" ); is( $z == '1/33', 1, "z == '1/33'" ); ok( $x > $y, "x > y" ); ok( $x > $z, "x > z" ); ok( $y > $z, "y > z" ); ok( $y < $x, "y < x" ); ok( $z < $x, "z < x" ); ok( $z < $y, "z < y" ); is( $x <=> $y, 1, "x <=> y" ); is( $x <=> $z, 1, "x <=> z" ); is( $y <=> $z, 1, "y <=> z" ); is( $x <=> $x, 0, "x <=> x" ); is( $y <=> $y, 0, "y <=> y" ); is( $z <=> $z, 0, "z <=> z" ); is( $y <=> $x, -1, "y <=> x" ); is( $z <=> $x, -1, "z <=> x" ); is( $z <=> $y, -1, "z <=> y" ); is( $x cmp $y, 1, "x cmp y" ); is( $x cmp $z, 1, "x cmp z" ); is( $y cmp $z, 1, "y cmp z" ); is( $x cmp $x, 0, "x cmp x" ); is( $y cmp $y, 0, "y cmp y" ); is( $z cmp $z, 0, "z cmp z" ); is( $y cmp $x, -1, "y cmp x" ); is( $z cmp $x, -1, "z cmp x" ); is( $z cmp $y, -1, "z cmp y" ); ok( 0.33 < $x, "0.33 < x" ); ok( $x > 0.33, "x > 0.33" ); ok( '1/3' < $x, "'1/3' < x" ); ok( $x > '1/3', "x > '1/3'" ); is( $x == 0.5, 1, "x == 0.5" ); is( $y == 0.25, 1, "y == 0.25" ); is( $x eq 0.5, 1, "x eq 0.5" ); is( $y eq 0.25, 1, "y eq 0.25" ); is( $x <=> 0.5, 0, "x <=> 0.5" ); is( $x <=> 0.4, 1, "x <=> 0.4" ); is( $x <=> 0.6, -1, "x <=> 0.6" ); is( 0.5 <=> $x, 0, "0.5 <=> x" ); is( 0.4 <=> $x, -1, "0.4 <=> x" ); is( 0.6 <=> $x, 1, "0.6 <=> x" ); is( $x cmp 0.5, 0, "x cmp 0.5" ); is( $x cmp 0.4, 1, "x cmp 0.4" ); is( $x cmp 0.6, -1, "x cmp 0.6" ); is( 0.5 cmp $x, 0, "0.5 cmp x" ); is( 0.4 cmp $x, -1, "0.4 cmp x" ); is( 0.6 cmp $x, 1, "0.6 cmp x" ); #eof#
On Wed Mar 01 16:18:30 2006, DAVIDRW wrote: Show quoted text
> This is a patch for Number-Fraction-1.08 to add in the 'cmp' and '<=>' > (and thus through magic autogeneration all of lt, gt, le, ge, eq, ne > and <, >, <=, >=, ==, != ) operators for Number::Fraction objects. > > Number-Fraction-compare.txt contains a compare() sub w/pod and the > modified 'use overload' statement. > > Note: i made <=> and cmp be the same and do numeric comparison .. if > that's undesirable, compare() can be copied to string_compare() and > all the ->to_num() calls changed to ->to_string() calls. > > The second file comparison.t has a bunch of tests (and examples) to > verify that it's all working ..
Sorry, but I don't agree with this. I used to have 'cmp' and 'ncmp' methods in the module but in version 1.2 I added 'fallback => 1' which did away with the need for them. The code now works as expected (or, at least, as I expect!) Your new test file contained 47 tests. Of those, 15 failed when run against my version of the module. These fell into two categories. Five tests failed because you were doing a numerical comparison of a Number::Fraction object against a string. They gave a standard Perl warning that you were using a non-numeric argument to a numeric operator an then failed as the string was converted using Perl's standard string to number conversions (so, for example, "1/2" is evalutated as 1). I think that this is correct behaviour. "1/2" _is_ a string and shouldn't (by default) be interpreted as anything else. If you want strings like that to be considered as Number::Fraction objects then using the module in ':constants' mode does that. I was pleased to see that with the module in ':constants' mode then all of these tests passed. The other ten tests that failed were all using string comparision operators to compare either a Number::Fraction object to a number or two Number::Fraction objects. I think that these tests are wrong. Let's take for example $x eq 0.5 (where $x is a Number::Fraction object representing a half). Because you're explicitly using a string comparison operator then Perl compares the values as strings. "1/2" and "0.5" are not the same string. If you want to do a numeric comparison then you have the numeric operators. And they work as you'd expect. There are similar arguments for all of the other failed string comparison tests. By using 'fallback => 1', Number::Fraction objects act (as far as possible) like ordinary Perl scalars. I think that the behaviour that you're suggesting would potentially be confusing to people. For that reason I'm not going to add these changes. Hope that makes sense. Please let me know if you have any further questions. Cheers, Dave...
From: davidrw [...] cpan.org
On Thu Mar 02 07:32:30 2006, DAVECROSS wrote: Show quoted text
> > I think that this is correct behaviour. "1/2" _is_ a string and > shouldn't (by default) be interpreted as anything else.
I guess my line of thought was that anything that anything of the form: Number::Fraction <OPERATOR> <SOMETHING> Should be treated as: Number::Fraction <OPERATOR> (Number::Fraction->new(<SOMETHING>) || <SOMETHING>) For example, since N::F constructor takes "1/2", shouldn't "1/2" in the context of $fraction == "1/2" be treated as a fraction object? But i see your point about changing the default string behavior, and it probably is best to leave the defaults alone.. i guess i was just trying to be lazy and avoid doing a ->to_num on the left or a ->new("1/2") on the right ;) Show quoted text
> If you want > strings like that to be considered as Number::Fraction objects then > using the module in ':constants' mode does that. I was pleased to see > that with the module in ':constants' mode then all of these tests passed.
doh. i did see that when i first looked at the pod, and then it slipped my mind.. well, hacking source is more fun than RTFM'ing anyways, right? :) Yup, i think ':constants' does the trick for my scripts, too.. Show quoted text
> I think that these tests are wrong. Let's take > for example $x eq 0.5 (where $x is a Number::Fraction object > representing a half). Because you're explicitly using a string > comparison operator then Perl compares the values as strings. "1/2" and > "0.5" are not the same string.
Yeah, that bothered me too, though for my specific purposes it didn't matter.. That compare() method could be used for strings/cmp if to_num where changed to to_string .. Though if you're dealing with fraction objects, do you really ever want string comparison? Dunno (though probably, so that there's always the option) .. Show quoted text
> By using 'fallback => 1', Number::Fraction objects act (as far as > possible) like ordinary Perl scalars. I think that the behaviour that > you're suggesting would potentially be confusing to people. For that > reason I'm not going to add these changes.
no prob .. glad that the test script at least helped validate ':constants' and went to good use .. and thanks for the quick response & turnaround on the other two bugs -- i already grabbed 1.09! --david