• From: USA
  • Registered: 05/21/2009
  • Posts: 10

Topic: Rounding error in includes/ui/ui_view.inc -> price_in_words()

Hello,

Calling price_in_words() on the amount 1960.58 with user_price_dec set to 2 produces the string ''One Thousand Nine Hundred and Sixty and 57/100''  <-- Note ''57/100'' which should be ''58/100''.

Also, the amount 16418.29 produces ''Sixteen Thousand Four Hundred and Eighteen and 28/100''.  Again, note ''28/100'' instead of ''29/100''.

The following patch to ui_view.inc seems to fix the problem for me, but I'm not sure if it's the best fix.  Can you think of a better one?

--- ui_view-orig.inc    2012-02-07 13:24:33.398225655 -0500
+++ ui_view.inc    2012-02-07 13:24:36.218220917 -0500
@@ -774,7 +774,7 @@
     if ($dec > 0)
     {
         $divisor = pow(10, $dec);    
-        $frac = round2($amount - floor($amount), $dec) * $divisor;
+        $frac = (int) round2(($amount - floor($amount)) * $divisor, $dec);
         $frac = sprintf("%0{$dec}d", $frac);
         $and = _("and");
         $frac = " $and $frac/$divisor";

maasj's Website

  • joe
  • Administrator
  • Offline
  • Registered: 03/15/2007
  • Posts: 5,107

Re: Rounding error in includes/ui/ui_view.inc -> price_in_words()

Thanks for this patch. But for the amount of 845.14 your algorithm prints Eight hundred and Fourty_five and 13/100.
With the old algorithm it was printed ok. So we will have to improve the algorithm a bit.

/Joe

  • joe
  • Administrator
  • Offline
  • Registered: 03/15/2007
  • Posts: 5,107

Re: Rounding error in includes/ui/ui_view.inc -> price_in_words()

Due to a bug in sprintf replacing the if statement solves the problem:

    if ($dec > 0)
    {
        $divisor = pow(10, $dec);
        $frac = round2($amount - floor($amount), $dec) * $divisor;
        $frac = sprintf("%0{$dec}d", round2($frac, 0));
        $and = _("and");
        $frac = " $and $frac/$divisor";
    }

The HG repository will be updated.

/Joe

  • From: USA
  • Registered: 05/21/2009
  • Posts: 10

Re: Rounding error in includes/ui/ui_view.inc -> price_in_words()

Hi Joe,

joe wrote:

Due to a bug in sprintf replacing the if statement solves the problem:

Thanks for the new patch, it seems to work well for me!  And I don't think there's a bug in sprintf(), it's just a known problem that PHP (and other languages like Python) don't deliver proper math results by default when dealing with base 10 floating point numbers.  The real fix would be to use something like BC Math here which is slower (not enough to matter for this purpose) but delivers correct results all the time.  We ended up using BC Math for some of our custom FA reports because otherwise it's really flakey to do math on floats and then try to do an equality comparison with 0.0, for example.  Because the result of your floating point math will actually be something like 0.00000000000001239458345 if you printf() it with enough digits to see that.  Annoying!

-Jason

maasj's Website