Am I wrong or the service InvoiceServices.updatePaymentApplicationDefBd is used to both create a new payment application (why "update*"?) and update an existing one?

Now I probably understand one of the reasons why the implementation of InvoiceServices.updatePaymentApplicationDefBd is so complex.

Starting at line 2503 the service tries to fix things after that the user has choosen to update an existing payment application: for example, the user changes the invoiceId to which the payment application was associated.

In my opinion we should *not* really handle things in this way.
I mean that we should not expose the updatePaymentApplication service to the users: the only actions allowed to a user should be:
* create a new payment application
* cancel/remove an existing payment application
Our code will be much more cleaner and simple, and, even if the user will have to perform two tasks instead of one, it will be more correct from a procedural point of view.

Jacopo

PS: I really think that a service implementation should never exceed a certain amount of lines of code; by experience, if it is too long then there is probably an issue in the logic or implementation... of course I'm joking



Jacopo Cappellato wrote:
To the authors, or other developers comfortable with the logic implemented in the service:

InvoiceServices.updatePaymentApplicationDefBd(...)

could you please help to review the patch committed in

https://issues.apache.org/jira/browse/OFBIZ-1144

?

Or help me to understand the logic implemented in the service?

I really cannot understand much of the stuff in it and so it is difficult for me to review the important fix in OFBIZ-1144.

Thanks,

Jacopo


Reply via email to