[Gnucash-changes] r13248 - gnucash/trunk/src/calculation - Fix Bug 107876 - financial calculator would call exit(1) if calculation resulted in an interest rate of zero.

[Neil Williams]

On Monday 13 February 2006 2:08 am, Derek Atkins wrote:
> Quoting Neil Williams <codehelp at cvs.gnucash.org>:
> > _C (double eint, double pmt, unsigned beg)
> > {
> > -  return pmt * _B (eint, beg);
> > +  unsigned check = _B (eint, beg);

> I'm not convinced this patch is correct.  _B() returns a double,

I'm setting up an error state that is perpetuated through the code. If these 
are converted back to doubles, gnucash goes into an infinite loop. I'll do 
something about the data loss but we have to break the loop too.

I started looking at this just as you did - but eint == 0.0 must be treated as 
a special case or the whole thing gets v.ugly.

> but 
> you're assigning it to an unsigned -- so you're going to lose data
> in the case.  

Once you get to eint == 0.0 being true, the game is up, the code must simply 
shutdown ASAP. The code becomes unstable and far more serious data loss is 
inevitable. Data loss within the calculator at that point is largely 
irrelevant.

This is why exit(1) was being called originally - brute force error handling.
:-(

I've set a wrapper so that the data is preserved in the normal case but the 
error state itself does need to be an unsigned int.

> I also don't see why it matters here; there's nothing 
> illegal about multiplying by zero.

You'd have thought that, (I did), but reverting causes a permanent loop to be 
setup.

All these zero checks are nothing to do with the calculation, these are 
intended only as an error state and all code is to abort when this state is 
detected. There is no point worrying about data that may need to be 
calculated once eint is 0.0, it cannot be done. We just get out of the code 
as fast as we can.

> You're also inconsistent with your checks.  Sometimes you say:
>   if (foo)

The next line takes care of if (foo == 0.0) in the ? : ; conditional. We just 
don't want this line executing:
CC = (CC - fv) / (CC + pv);
if CC == 0.0
I know it should be OK because of the +pv but the code just keeps on looping. 
we can't risk _any_ calculation once eint == 0.0.

> > +  if(CC == 0) return 0.0;
>
> And yet another inconsistency --  == 0 instead of == 0.0.

Sorry about that, got carried away with the int error state. Fixed.

> Are you sure this code even still works?  Is there a test case that shows
> the actual bug(s)?

Yes and yes, in the bug report.
http://bugzilla.gnome.org/show_bug.cgi?id=107876

Enter any set of figures that would result from a zero-interest loan.

e.g. Clear interest rate, clear future value and payment amount. Set payment 
amount to a simple denominator of the loan amount (500) and future value to 
zero. Click calculate.

Gnucash would simply shutdown with exit(1) - offering no chance to save the 
current data file - hence the original bug. Now the calculator aborts and 
displays an interest rate of '-' and gnucash keeps running. It's almost 
intuitive, as errors go.
;-)

-- 

Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20060213/42330843/attachment.bin