Multiplication problem

John Vanbeek has asked for the wisdom of the Perl Monks concerning the following question:

Hi Monks,

I'm using a webshop program called hishop. Recently a discovered a bug. The problem is that when an article has a price of 17.95 and the quantity is 3 the program makes a multiplication error. The total should be 53.85, but the program gives 53.84. Using a price of 17.96, 17.94 or 10.95 gives the correct result.

The program uses a subprogram called round(), this is where it goes wrong. My knowledge of perl is not sufficient to find the error.

I made an example program. Please help.

Best regards, John Vanbeek

 

#!/usr/bin/perl 

$price=17.95; 
$qtyx=3; 

$cost=round($price * $qtyx); 


$cost=commas($cost);$price=round($price);$price=commas($price); 

print "Content-type: text/html\n\n"; 

print "total costs are $cost"; 


sub round { 
my($fig)=@_; 
$fig=($fig * 100); 
if($fig=~/\./){ 
$fig=~m!^([0-9]*)\.([0-9])!;$fig=$1;$ext=$2; 
if($ext>4){$fig++;} 
} # end if($fig=~/\./); 
$fig=int($fig); if($fig=~/^[0-9][0-9]$/){$fig="0$fig";} 
$fig=~s!^([0-9]+)([0-9][0-9])$!\1\.\2!; 
if($fig eq '0'){$fig='0.00';} 
return($fig); 
} 
######################################################################
+####################### 
sub commas { my($fig)=@_; $fig=~s!\,!!g; 
my ($figx,$ext)=split(/\./,$fig); $ext=($ext)?"\.$ext":''; 
$figx=~s!^([0-9]+)([0-9]{3})$!\1\,\2!; 
while($figx=~/^([0-9]+)([0-9]{3})\,(.*?)$/){$figx=~s!^([0-9]+)([0-9]{3
+})\,(.*?)$!\1\,\2\,\3!;} 
$figx=($figx.$ext); 
return $figx; 
}
[download]

Comment on Multiplication problem Download Code

Replies are listed 'Best First'.
Re: Multiplication problem by mwah (Hermit) on Dec 10, 2007 at 15:23 UTC
Your "formula" seems overly complicated (and probably broken). For the start, I'd go with a quick&dirty simple & minimal variant and advance from there: `#!/usr/bin/perl use strict; use warnings; my @prices = qw'.1 17.95 0 10000 0.00 21 .22'; my $qtyx = 3; print "Content-type: text/html\n\n"; print map "$_ x $qtyx, total costs are: ". VIEWPRICE( MAKEPRICE($_)*$qtyx + ) .'<br />', @prices; sub MAKEPRICE { (my $mp = sprintf "%.2f", (shift)) =~ tr/.//d; return $mp } sub VIEWPRICE { my $vp = sprintf "%03d", (shift); $vp =~ s/(..)$/.$1/ or $vp = sprintf "%.2f",$vp; return $vp }` [download] This prints (<br /> translated): `Content-type: text/html .1 x 3, total costs are: 0.30 17.95 x 3, total costs are: 53.85 0 x 3, total costs are: 0.00 10000 x 3, total costs are: 30000.00 0.00 x 3, total costs are: 0.00 21 x 3, total costs are: 63.00 .22 x 3, total costs are: 0.66` [download] Regards mwa	[reply] [d/l] [select]
A reply falls below the community's threshold of quality. You may see it by logging in.
Re: Multiplication problem by dragonchild (Archbishop) on Dec 10, 2007 at 14:50 UTC
Replace round() with: `sub round { my ($num) = @_; return int( $num * 100 ) / 100; }` [download] There's a much simpler commas() function, too, but I wouldn't replace what isn't broken. My criteria for good software: Does it work? Can someone else come in, make a change, and be reasonably certain no bugs were introduced?	[reply] [d/l]
Re^2: Multiplication problem by John Vanbeek (Initiate) on Dec 10, 2007 at 15:20 UTC
Thanks for your reply, but the outcome is the same. The multiplication is still incorrect. Any other suggestions? gr. John	[reply]
Re^3: Multiplication problem by dragonchild (Archbishop) on Dec 10, 2007 at 15:44 UTC
Huh. You're absolutely correct. Floating point match is notoriously problematic when doing rounding. It's just the nature of doing floating point arithmetic on a binary processor. You could take mwah's solution. Or, you could convert everything to integers. So, instead of working in dollars, you work in cents. `my $price = 1795; # Cents, not dollars my $qtyx = 3; # Amount, not a price. my $cost = $price * $qtyx; my $cost_to_display = $cost / 100;` [download] And that works when tested. My criteria for good software: Does it work? Can someone else come in, make a change, and be reasonably certain no bugs were introduced?	[reply] [d/l]
Re^4: Multiplication problem by John Vanbeek (Initiate) on Dec 10, 2007 at 16:28 UTC
Re^5: Multiplication problem by dragonchild (Archbishop) on Dec 11, 2007 at 20:56 UTC
Re: Multiplication problem by goibhniu (Hermit) on Dec 10, 2007 at 16:28 UTC
I'm not sure how much freedom you have to refactor, but you might consider working with integers in pennies instead of floating points in dollars. Also, while I know nothing of the larger context of your program, I'm not sure you need to round when multiplying an integer quantity (one assumption I'm making). Rounding would be more of a concern in division or multiplying two floats (do you have fractional quantities?). Consider: `C:\chas_sandbox>perl -le "print +(int((17.95 3)100 )/ 100)" 53.84 C:\chas_sandbox>perl -le "print 17.95 3" 53.85 C:\chas_sandbox>perl -le "print 1795 3" 5385` [download] Whatever the case, it's probably wise to follow dragonchild's advice and not fix what isn't broke. I humbly seek wisdom.	[reply] [d/l]
Re: Multiplication problem by oha (Friar) on Dec 10, 2007 at 16:51 UTC
why you don't simply use cents as the base units and when you print out data you just: `sub tostring { my $cent = "00". shift; $cent =~ s/(\d\d)$/.$1/; $cent =~ s/^0+(\d)/$1/; 1 while($cent =~ s/(\d)(\d\d\d[.'])/$1'$2/); $cent; } print tostring(2)." + ".tostring(99998)." = ".tostring(100000)."\n"; # => .02+999.98=1'000.00` [download] Oha	[reply] [d/l]
Re: Multiplication problem by rgiskard (Hermit) on Dec 10, 2007 at 18:01 UTC
I'm adding my 2 cents about rounding and versions of perl, as it seems you have your solution but didn't mention your perl version. If you're running Perl 5.6.X verses something greater than 5.8; you'll find that the math(rounding) may be different. This was certainly true for sprintf and rounding between 5.6 and 5.8 (See Perl5.8 delta: Understanding of numbers).	[reply]
Re: Multiplication problem by pc88mxer (Vicar) on Dec 10, 2007 at 18:45 UTC
This really is a problem with using floating-point arithmetic. Programs which deal with prices should really use decimal arithmetic to get exact results. Or, equivalently, express prices as integers in the lowest possible units. For example, express prices in cents instead of dollars. Here's a page which describes other reasons why and when decimal arithmetic is preferred: http://www2.hursley.ibm.com/decimal/decifaq.html	[reply]
Re: Multiplication problem by Roy Johnson (Monsignor) on Dec 10, 2007 at 17:00 UTC
I think that, for your relatively simple, low-precision use, you would get what you want with `sub round { sprintf "%.02f", @_ }` [download] Caution: Contents may have been coded under pressure.	[reply] [d/l]
Re: Multiplication problem by swampyankee (Parson) on Dec 11, 2007 at 00:07 UTC
As other posters have said, floating point is fraught with peril (see "What Every Computer Scientist Should Know About Floating-Point Arithmetic". Conversely, your examples are only four significant figures, and nowhere near the extremes that can be represented in doubles, so it's not unreasonable to expect the right answer. emc Information about American English usage here and here. Any Northeastern US area jobs? I'm currently unemployed.	[reply]