fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] ptuomola commented on a change in pull request #1184: FINERACT-1089 removing the rs.previous()
Date Sat, 18 Jul 2020 03:19:02 GMT

ptuomola commented on a change in pull request #1184:
URL: https://github.com/apache/fineract/pull/1184#discussion_r456741939



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountInterestRateChartReadPlatformServiceImpl.java
##########
@@ -305,12 +305,8 @@ public DepositAccountInterestRateChartSlabData extractData(ResultSet
rs) throws
                     if (incentiveData != null) {
                         chartSlabData.addIncentives(incentiveData);
                     }
-                } else {
-                    rs.previous();

Review comment:
       OK so looks like we have two calls to rs.previous(), not just one. 
   
   As far as I can see, this function is trying to combine all rows with the same interestRateChartSlabId
to a single chartSlabData. So when you get a row with a different ID, then it backs the cursor
by one to ensure the calling function gets the right row when it calls next(). Just removing
this rs.previous() will not work, as then the cursor is in the wrong position. 
   
   We need to think how to fix this. Personally I think this "manipulating ResultsSet in different
functions and hoping that each function returns it in the right state" is not a very clean
approach, but I'm not sure it's worth a complete rewrite - probably just easier to find some
quick fix. 




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

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



Mime
View raw message