adamsaghy commented on code in PR #4202:
URL: https://github.com/apache/fineract/pull/4202#discussion_r1900412535


##########
fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java:
##########
@@ -1594,12 +1594,19 @@ public void 
validateAccountBalanceDoesNotBecomeNegative(final String transaction
     public void validateAccountBalanceDoesNotViolateOverdraft(final 
List<SavingsAccountTransaction> savingsAccountTransaction,
             final BigDecimal amountPaid) {
         if (savingsAccountTransaction != null) {
-            SavingsAccountTransaction savingsAccountTransactionFirst = 
savingsAccountTransaction.get(0);

Review Comment:
   Would you mind to simplify the logic a little bit further?
   
   You can merge the `savingsAccountTransaction != null` and 
`savingsAccountTransaction.size() > 0` condition together and if they are TRUE 
take the running balance of the first element otherwise it is BigDecimal.ZERO 
(if there are no any transaction the running balance must be 0). 
   After you can deduct the `amountPaid` from the above balance and if the 
balance is negative and `allowOverdraft` is FALSE then 
`InsufficientAccountBalanceException` to be thrown.
   
   I believe it would be easier to read and cover better the requirements!
   
   Also kindly asking you to write a test (it can be unit tests), which covers 
the following situations:
   - `savingsAccountTransaction` is empty and `amountPaid` is positive and 
`allowOverdraft = true` -> **No exception!**
   - `savingsAccountTransaction` is empty and `amountPaid` is positive and 
`allowOverdraft = false` -> **Exception to be thrown!**
   - `savingsAccountTransaction` is not empty and `amountPaid` is positive and 
`amountPaid` is higher than the running balance of the 1st transaciton and 
`allowOverdraft = true` -> **No exception!**
   - `savingsAccountTransaction` is not empty and `amountPaid` is positive and 
`amountPaid` is lower than the running balance of the 1st transaction and 
`allowOverdraft = false` -> **Exception to be thrown!**
   - `savingsAccountTransaction` is not empty and `amountPaid` is positive and 
`amountPaid` is equal with the running balance of the 1st transaction and 
`allowOverdraft = false` -> **No exception!**



-- 
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]

Reply via email to