Analysis of payment_post()

[Erik Huelsmann <..hidden..>]

Hi Chris, others,

In order to completely fix payment_post(), I need some help. I'm
listing my questions below.

1. the function takes an in_curr argument. I assume this is to be the
currency of the payment transaction. The remainder of the function
goes on to assume this is the same currency as the currency of the
original AR/AP item. You can see that from the fact that it uses
in_curr (through current_exchangerate) as a parameter in the FX
gain/loss calculation. However, if the in_curr currency is required to
be the same as the original transaction, why have it? If it's not,
doesn't that unnecessarily complicate things? (It's possible, sure.)

2. if the current exchange rate of the in_curr currency is exactly 1,
the function assumes that's because the payment (and thus the original
transaction) are happening in the default currency. As a consequence,
the historic rate is also set to 1. Isn't this a dangerous assumption?
Should a currency ever happen to have an exact 1-1 rate (for example
by design), then this assumption fails to hold.

3. currently, the gain/loss block contains the same query 4 times,
with different arguments. Wouldn't it be nicer (more
understandable/maintainable) to query the arguments first and then
have a single query get or insert the values we're looking for?

4. why does the IF (upper_array(in_cash_account, 1) > 0) THEN ... END
IF block have two loops, both looping over the in_transaction_id
array? Having 2 loops could mean we're calculating the same values
with the risk of calculating them in different ways.

5. having the CASE forms all over the place selecting between the AR
and AP cases, is very annoying as well. Can't we design the function
(and many like it in Payment.sql) to have a minimum of AR/AP specific
code?


That do others think?


Bye,

Erik.