ImalkaPerera commented on PR #6664: URL: https://github.com/apache/fineract/pull/6664#issuecomment-6262078682
> Hi @ImalkaPerera, thanks for the PR. The guard is the right fix for the division by zero on the last installment. > > One question: on the last installment we now drop `principalPerPeriod - currentPeriodFixedPrincipalAmount`. Who picks that difference up? If the generator does not force the last installment to the outstanding balance, the principal portions won't add up to the loan principal. An assertion on the sum in the test would settle it. > > Nit: the first test calls `calculateTotalPrincipalForPeriod` twice on a stateful object (`adjustPrincipalForFlatLoans`), so the second call is not on clean state, and the `assertThatCode` is redundant. Thanks for the review, @galovics. Good catch! Regarding your question, the leftover doesn't get lost. On the last installment, `adjustPrincipalIfLastRepaymentPeriod` adjusts the principal so that the total adds up to the loan principal. I've added an assertion to the test to verify that the principal portions sum to the loan principal. I also addressed the nit. The test now runs the periods in order instead of calling the method twice on the same stateful object, and I removed the redundant `assertThatCode`. I've pushed the updates. Please take another look when you get a chance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
