[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.

[Christian Stimming]

Hi Neil,

Neil Williams schrieb:
>>In this fin.c 
>>case, I don't understand why a different solution isn't possible --
> 
> Because unless the return value is set to the error state, the recursive loop 
> becomes infinite.
> 
> Hopefully, using g_return_val_if_fail will make it clear that returning 0.0 
> from functions that calculate interest rates indicates a failure.

Well, the code clearly is much better now, because you don't use any 
casts on the return value anymore.

However, I think you got the condition in g_return_val_if_fail() the 
wrong way --

@@ -1246,5 +1247,5 @@
    /* if eint == 0.0, all processing _must_ stop or
  	a recursive loop will start. */
-  if (eint == 0.0) { return 0.0; }
+  g_return_val_if_fail(eint == 0.0, 0.0);

    return (1.0 + eint * (double) beg) / eint;

should probably rather be
    g_return_val_if_fail(eint != 0.0, 0.0);
because the function is terminated if the expression is not true. And in 
any case, I thought that at least in the function _B() you could as well 
write
    g_assert(eint != 0.0);
which would terminate the program here, but that's okay because the 
parameter eint is now checked *before* calling _B(), so this assertion 
should always be true in the current code. In _C() the call 
g_return_val_if_fail() might be the sufficient solution as long as the 
calling parameter cannot be checked in all calls of _C().

Are you sure that g_assert() 
http://developer.gnome.org/doc/API/2.0/glib/glib-Warnings-and-Assertions.html#g-assert 
didn't terminate the program immediately if the assertion failed? It 
should; otherwise something might be wrong in your compiler setup.

Anyway, the important part was that the check for a nonzero denominator 
is now done before calling _B(), and that's surely the better way to 
check this. Thanks for fixing this.

Christian