[GitHub] [fineract] adamsaghy commented on a diff in pull request #2569: FINERACT-1721: Delinquency classification as a Loan COB business step

[GitBox <gi...@apache.org>]

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


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -148,7 +148,7 @@ public CommandProcessingResult deleteDelinquencyBucket(Long delinquencyBucketId,
     public CommandProcessingResult applyDelinquencyTagToLoan(Long loanId, JsonCommand command) {
         Map<String, Object> changes = new HashMap<>();
 
-        final Loan loan = this.loanRepository.findOneWithNotFoundDetection(loanId);

Review Comment:
   As the loan is not used as a reference, rather doing actions on it, we should use the "this.loanRepository.findOneWithNotFoundDetection"
   
   Using the "getReferenceById" is only useful if we dont need to fetch the loan entirely (fields and associations), but only we need the reference of it. 
   Example:
   `new LoanTransaction(loan, transactionDate,....)`
   
   Here we dont need to fetch the loan, only the reference which can be used to associate the loan entity with the loan transaction entity.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -329,7 +329,7 @@ private Map<String, Object> lookUpDelinquencyRange(final Loan loan, final Delinq
     }
 
     public Map<String, Object> setLoanDelinquencyTag(Long loanId, Long delinquencyRangeId) {
-        final Loan loan = this.loanRepository.findOneWithNotFoundDetection(loanId);
+        final Loan loan = this.loanRepository.getReferenceById(loanId);

Review Comment:
   Nice catch! We dont need to fetch the full loan here. Reference is enough! :)



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -166,7 +166,7 @@ public CommandProcessingResult applyDelinquencyTagToLoan(Long loanId, JsonComman
 
     @Override
     public void applyDelinquencyTagToLoan(Long loanId, Long ageDays) {
-        final Loan loan = this.loanRepository.findOneWithNotFoundDetection(loanId);

Review Comment:
   As the loan is not used as a reference, rather doing actions on it, we should use the "this.loanRepository.findOneWithNotFoundDetection"
   
   Using the "getReferenceById" is only useful if we dont need to fetch the loan entirely (fields and associations), but only we need the reference of it. 
   Example:
   `new LoanTransaction(loan, transactionDate,....)`
   
   Here we dont need to fetch the loan, only the reference which can be used to associate the loan entity with the loan transaction entity.



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org