AUDIT: r17421 - gnucash/trunk/lib/libqof/qof - Add a new function to the gnc_numeric library that converts denominators to exact powers of ten.

[Chris Shoemaker]

On Thu, Sep 18, 2008 at 06:12:19AM -0700, Charles Day wrote:
> On Tue, Sep 16, 2008 at 2:40 PM, Charles Day <cedayiv at gmail.com> wrote:
> 
> > On Tue, Sep 16, 2008 at 10:54 AM, Charles Day <cedayiv at gmail.com> wrote:
> >
> >> On Tue, Sep 16, 2008 at 9:38 AM, Andreas Köhler <andi5.py at gmx.net> wrote:
> >>
> >>> Hi,
> >>>
> >>> On Mon, 2008-09-01 at 22:09 +0200, Andreas Köhler wrote:
> >>> > Hi Charles,
> >>> >
> >>> > On Sun, 2008-07-27 at 11:33 -0400, Charles Day wrote:
> >>> > > Trac: http://svn.gnucash.org/trac/changeset/17421
> >>> > > Log:
> >>> > > Add a new function to the gnc_numeric library that converts
> >>> denominators to exact powers of ten.
> >>> >
> >>> > > Modified: gnucash/trunk/lib/libqof/qof/gnc-numeric.c
> >>> > > ===================================================================
> >>> > > --- gnucash/trunk/lib/libqof/qof/gnc-numeric.c      2008-07-27
> >>> 15:11:19 UTC (rev 17420)
> >>> > > +++ gnucash/trunk/lib/libqof/qof/gnc-numeric.c      2008-07-27
> >>> 15:33:23 UTC (rev 17421)
> >>> > > +  fraction = converted_val.denom;
> >>> > > +  if (fraction <= 0)
> >>> > > +    return FALSE;
> >>> >
> >>> > out of curiosity, what is the reason for disallowing negative
> >>> > denominators?  I am not sure whether there is a gnc_numeric API
> >>> function
> >>> > to switch signs of nom and denom safely, but I am sure that doing it is
> >>> > possible :-) But maybe the current behavior is actually desired.
> >>>
> >>> I just noticed that ignoring negative denominators means that 1/(-10) is
> >>> now printed as "1 * 10", where it was printed as "10.00" before.  Is
> >>> that a regression we have to fix or not?
> >>>
> >>
> >> I can see how that would happen, because in previous versions, non-decimal
> >> fractions were forced to print in decimal form in some places. So even
> >> though the old routines considered 1/(-10) non-decimal, it would print it as
> >> decimal anyway. I think this takes us back to the original discussion.
> >> Should go ahead and add support for negative denominators to
> >> gnc_numeric_to_decimal()? I must only take less than 5 lines of code.
> >>
> >
> > Here's the first attempt at that change. How are you testing these numbers?
> >
> 
> I went ahead and committed this change in r17554. Calling
> gnc_numeric_to_decimal on 4/-3 now gets you 12. However, there is an
> overflow danger because gnc_numeric_convert()  doesn't check for that
> (apparently this has been a problem for some time). Someone else should
> probably look at that, as I am no expert on 64-bit/128-bit math.

I haven't followed the thread at all, but just in case anyone's not
aware, I thought I should remind everybody about the reciprocal
denominator represention.  IIRC, signs are always stored on the
numerator, and a negative denominator represents a reciprocal
denominator (essentially a multiplication).  Last I knew, this
representation is used internally by some of the operations.

-chris

> 
> -Charles
> 
> 
> > $ svn diff
> > Index: gnc-numeric.c
> > ===================================================================
> > --- gnc-numeric.c       (revision 17505)
> > +++ gnc-numeric.c       (working copy)
> > @@ -1043,7 +1043,14 @@
> >
> >    converted_val = *a;
> >    if (converted_val.denom <= 0)
> > -    return FALSE;
> > +  {
> > +    converted_val = gnc_numeric_convert(converted_val, 1,
> > GNC_DENOM_EXACT);
> > +    if (gnc_numeric_check(converted_val) != GNC_ERROR_OK)
> > +      return FALSE;
> > +    *a = converted_val;
> > +    *max_decimal_places = decimal_places;
> > +    return TRUE;
> > +  }
> >
> >    /* Zero is easily converted. */
> >    if (converted_val.num == 0)
> >
> >
> >>
> >>
> >>> Ciao,
> >>> -- andi5
> >>>
> >>
> >> Cheers,
> >> Charles
> >>
> >
> >
> _______________________________________________
> gnucash-devel mailing list
> gnucash-devel at gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel