budaidev commented on code in PR #6264:
URL: https://github.com/apache/fineract/pull/6264#discussion_r3881567780
##########
fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/domain/WorkingCapitalLoanPeriodPaymentRateChange.java:
##########
@@ -40,6 +41,20 @@
@Table(name = "m_wc_loan_period_payment_rate_change")
public class WorkingCapitalLoanPeriodPaymentRateChange extends
AbstractAuditableWithUTCDateTimeCustom<Long> {
+ /** Two rate changes on the same loan can differ only from the ninth
decimal of the EIR onwards. */
+ private static final int EIR_SCALE = 12;
+
+ private static final int AMOUNT_SCALE = 6;
+
+ /** The annualised rate is a headline figure; six decimals match the rate
columns either side of it. */
+ private static final int ANNUAL_EIR_SCALE = 6;
+
+ /**
+ * Fixed rather than the tenant's money rounding mode: these are rates and
derived figures, not money, and the whole
+ * point of pinning the scales here is that the same inputs yield the same
stored value for every tenant.
+ */
+ private static final RoundingMode SNAPSHOT_ROUNDING = RoundingMode.HALF_UP;
Review Comment:
Agreed — changed to `HALF_EVEN`.
The reason the constant exists at all is unchanged: it is pinned rather than
taken from `MoneyHelper.getRoundingMode()` so the stored value doesn't vary
with tenant configuration. So this settles *which* fixed mode, and banker's
rounding is the better answer for exactly the reason you give — it doesn't
drift upward across a series of ties. I updated the comment to say so rather
than leaving the choice unexplained.
Value-neutral for what the suite asserts: the snapshot figures are pinned at
scales (12 / 6 / 6) where none of them lands on an exact tie, so the tests are
green either way. The change is about not encoding a biased default, not about
a number that moves today.
Two neighbouring `HALF_UP` uses are deliberately left alone —
`ProjectedAmortizationScheduleMapper.DISPLAY_ROUNDING` and
`WorkingCapitalLoanAccountDataMapper.toAvroDecimalScale`. Both predate this PR
and neither is on the snapshot path, so folding them in felt like unrelated
behaviour riding along. Happy to switch them in a follow-up if the convention
is meant to cover them too.
--
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]
