I need help to review updatePaymentApplicationDefBd service...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

I need help to review updatePaymentApplicationDefBd service...

Jacopo Cappellato
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 | Threaded
Open this post in threaded view
|

Re: I need help to review updatePaymentApplicationDefBd service...

Jacopo Cappellato
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