gnc_numeric: fractional displays and rounding

[Charles Day]

On Fri, Jul 25, 2008 at 10:07 AM, Christian Stimming <stimming at tuhh.de>wrote:

> Am Freitag, 25. Juli 2008 20:15 schrieb Charles Day:
> > So my first question: Is there an existing routine which will take a
> > gnc_numeric and attempt to change its denominator to any exact power of
> > ten?
>
> No, there's not. Feel free to add one.
>
> Also, the whole issue with rational numbers was probably an overkill
> solution
> anyway. Of course binary floating point numbers (float/double) are the
> wrong
> thing to do in financial calculations, but rational numbers are providing
> more flexibility than needed. What should have been implemented at the time
> are decimal (!) floating point numbers. I.e. instead of two large integers
> as
> nominator and denominator (the current rational numbers), we should have
> used
> one large integer for the mantissa m and a small integer e for the position
> of the decimal point, so that the resulting number is m * 10^(e)  .  This
> is
> different from "normal" floating point types which are always using a power
> of two because of the machine's number representation in binary. But
> whatever. Rational numbers is what we have right now, so that's what we
> will
> stick with.
>
> > The second thing I wanted to ask about is rounding. Currently many prices
> > going into the price db experience rounding, and this causes bugs (such
> as
> > bug 309863.)  Does anyone know WHY prices are being rounded? I suspect
> that
> > this rounding is unnecessary and is being done just to avoid fractional
> > display (see above).
>
> No idea.
>
> > If there were a routine that converted denominators to any power of ten,
> > but only without rounding, then the price editor could use that on prices
> > to make as many as possible stored in a way that prints in decimal form.
>
> As prices from actual transactions involve the division of numbers, I'm
> afraid
> the requirement "without rounding" might not be feasible in that case
> anymore. There might be rounding with much more precision than what
> currently
> happens, though, so I completely agree there is much room for improvement
> here.
>
> > Alternatively, the gnc_numeric printing routines such as
> xaccPrintAmount()
> > use this conversion purely for printing purposes; this would avoid all
> > unnecessary fractional display of gnc_numerics with only a small
> > performance hit (I say "small" because these routines do most of the work
> > already and just avoid the last few steps). Which way is preferred?
>
> I think changing xaccPrintAmount() the way you described here is a good
> idea
> in any case and should be implemented once you have that routine.
>

A patch for printing is shown below. However, since the new
convert_to_decimal function may be useful outside of printing, should I move
it into gnc-numeric.c? And if so, what name would you like? Do you like the
style of the parameters? The idea is that you pass in a pointer to your
gnc_numeric, and convert_to_decimal() returns TRUE if your gnc_numeric has
been converted. Otherwise FALSE is returned and your gnc_numeric is
unchanged.

> Finally, I have already written such a routine. I hope that I have not
> > reinvented the wheel...
>
> Good to hear! Thanks for your work!
>
> Christian
>

-Charles


Index: gnc-ui-util.c
===================================================================
--- gnc-ui-util.c       (revision 17402)
+++ gnc-ui-util.c       (working copy)
@@ -1045,6 +1045,68 @@
 }

 static gboolean
+convert_to_decimal(gnc_numeric *val, guint8 *max_decimal_places_p)
+{
+  guint8 max_decimal_places = 0;
+  gnc_numeric converted_val;
+  gint64 fraction;
+
+  g_return_val_if_fail(val, FALSE);
+
+  converted_val = *val;
+  fraction = converted_val.denom;
+  if (fraction <= 0)
+    return FALSE;
+
+  while (fraction != 1)
+  {
+    switch (fraction % 10)
+    {
+      case 0:
+        fraction = fraction / 10;
+        break;
+
+      case 5:
+        converted_val = gnc_numeric_mul(converted_val,
+                                        gnc_numeric_create(2, 2),
+                                        GNC_DENOM_AUTO,
+                                        GNC_HOW_DENOM_EXACT |
+                                        GNC_HOW_RND_NEVER);
+        if (gnc_numeric_check(converted_val) != GNC_ERROR_OK)
+          return FALSE;
+        fraction = fraction / 5;
+        break;
+
+      case 2:
+      case 4:
+      case 6:
+      case 8:
+        converted_val = gnc_numeric_mul(converted_val,
+                                        gnc_numeric_create(5, 5),
+                                        GNC_DENOM_AUTO,
+                                        GNC_HOW_DENOM_EXACT |
+                                        GNC_HOW_RND_NEVER);
+        if (gnc_numeric_check(converted_val) != GNC_ERROR_OK)
+          return FALSE;
+        fraction = fraction / 2;
+        break;
+
+      default:
+        return FALSE;
+    }
+
+    max_decimal_places += 1;
+  }
+
+  if (max_decimal_places_p)
+    *max_decimal_places_p = max_decimal_places;
+
+  *val = converted_val;
+
+  return TRUE;
+}
+
+static gboolean
 is_decimal_fraction (int fraction, guint8 *max_decimal_places_p)
 {
   guint8 max_decimal_places = 0;
@@ -1383,7 +1445,7 @@
   /* at this point, buf contains the whole part of the number */

   /* If it's not decimal, print the fraction as an expression */
-  if (!is_decimal_fraction (val.denom, NULL))
+  if (!convert_to_decimal(&val, NULL))
   {
     if (!gnc_numeric_zero_p (val))
     {