Bug 103181: not-monthly repayment schedules not supported in Mortgage/Loan Druid

[John Ralls]

On Jan 13, 2014, at 6:10 AM, alessandro basili <alessandro.basili at cern.ch> wrote:

> Hi there,
> 
> I've just registered to this list since I'd like to fix this bug but I'd
> like to first share some thoughts before hitting the ground running.
> 
> As of now it looks like only monthly repayment is 'supported' but this
> is only by 'accident' to me. Looking at the code
> (src/gnome/assistant-loan.c) it seems there's provision to support:
> 
> typedef enum
> {
>  GNC_IRATE_SIMPLE,
>  GNC_IRATE_APR_DAILY,
>  GNC_IRATE_APR_WEEKLY,
>  GNC_IRATE_APR_MONTHLY,
>  GNC_IRATE_APR_QUARTERLY,
>  GNC_IRATE_APR_ANNUALLY
> }
> 
> repayments. I'm not sure though why we want to keep an enumerated type
> if the Mortgage/Loan Druid supports various combination of repayments.
> We may argue that no one will have a repayment plan each 19 days... but
> then I guess the Druid should not allow such flexibility.
> 
> Looking at the 'loan_apr_to_simple_formula' it seems that it deviates
> from the definition reported in the documentation
> (http://gnucash.org/docs/v2.4/C/gnucash-guide/loans_calcs1.html) since
> the payment frequency is hard coded to '1' (referred to as PF in the
> online-doc).
> 
> When, by chance, the frequency is 'monthly' the
> loan_apr_to_simple_formula, combined with the switch in
> 'loan_get_ipmt_formula' (and its siblings for pmt and ppmt), provides
> the correct ipmt formula, only because the parameter for compound_period
> is '1' and the divisor for ipmt is '12'.
> 
> TODO: a) or b)
> 
> a) fix loan_apr_to_simple_formula' with one that takes into account
> repayment periods and remove the second parameter in the g_string_printf
> function called by loan_get_ipmt_formula, since it will be taken into
> account by loan_apr_to_simple_formula.
> 
> b) leave unchanged loan_apr_to_simple_formula (even if wrong) and change
> the compound rate parameter in each call to be fixed at 1, while
> modifying the '12.0' to correspond to the actual compound frequency.
> 
> Considerations on the above mentioned proposals:
> 
> 1) considering that there's no provision in the current Mortgage/Loan
> Druid to provide a repayment frequency which is different from the
> compound frequency, I guess there's no real point to keep a
> repayment_period variable. On the other hand, a potential extension of
> the Druid to provide such feature would profit of the possibility to
> calculate the effective rate.
> 
> 2) fix a) will require an interface change for
> loan_apr_to_simple_formula since the repayment frequency will have to be
> provided as a parameter.
> 
> I've not yet tested any of the above, but if some of you might have an
> additional information I'm lacking of I would be happy to hear it before
> I move on.
> 
> On a side note, I find it is a little confusing that the online
> documentation is using some variable names like CF, PF and so on, while
> the code seems to drift with a completely different set of variable names.
> 
> I guess some work needs to be done somewhere else since I haven't found
> as of now where the compounding frequency is set in the druid.
> 
> BTW, I do not plan to be a developer or anything else, is just that I
> need this feature to keep track of my mortgage, that's why I'd like to
> press on with it.

I have no real opinion about the algorithms, but there are a bunch of bugs and complaints about the loan assistant (it's not called a druid anymore, and if that term is used somewhere in the 2.6 documentation, please file a bug), so a comprehensive rewrite that addresses all of them is preferable to addressing them piecemeal, especially if you need to make substantial architectural changes like replacing an enum with a continuous range.

The last substantial work on the nuts-and-bolts of that function was done in 2002 by a dev who's no longer active, so it's likely that there won't be much pushback on whatever changes you make as long as the results work and match the coding guidelines: http://wiki.gnucash.org/wiki/Development#Coding_Guidelines 

We're not terribly concerned about API changes, especially in the gnome and gnome-utils directories. Just make sure that if you do change a function's signature that you check for and fix both C and Scheme callers. We also need to separate the computations from the GUI code in those directories. You don't have to do that yourself, but please keep the goal in mind. If you can, please add unit tests to any functions you alter significantly.

Regards,
John Ralls