fix(transfer-intrabank): correct floating-point truncation in amount display - #2034
fix(transfer-intrabank): correct floating-point truncation in amount display#2034bhuvan-somisetty wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesAmount Formatting Precision
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
feature/transfer-intrabank/src/commonMain/kotlin/org/mifospay/feature/transfer/intrabank/hub/components/RecentPayeeCard.kt (1)
128-139: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRounding fix correctly addresses the truncation bug.
The cent-based rounding via
roundToInt()correctly resolves the ₹5.99/₹99.99/₹10.30 display issue described in the PR objective, since it rounds before splitting rather than truncating a raw double subtraction.One minor note:
centsis derived fromamount * 100as anInt, which overflows above ~21.4 million currency units. Given this is a recent-payee transfer amount, this is unlikely in practice, but usingroundToLong()would remove the ceiling entirely at negligible cost.♻️ Optional hardening
- val cents = (amount * 100).roundToInt() - val wholePart = cents / 100 - val decimalPart = cents % 100 + val cents = (amount * 100).roundToLong() + val wholePart = cents / 100 + val decimalPart = cents % 100🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/transfer-intrabank/src/commonMain/kotlin/org/mifospay/feature/transfer/intrabank/hub/components/RecentPayeeCard.kt` around lines 128 - 139, The amount formatting in formatAmount currently uses roundToInt() into an Int, which can overflow for very large transfer values. Update the cents calculation to use a wider integer type in RecentPayeeCard.formatAmount so the rounding fix remains safe for large amounts, while keeping the same whole-part/decimal-part formatting behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@feature/transfer-intrabank/src/commonMain/kotlin/org/mifospay/feature/transfer/intrabank/hub/components/RecentPayeeCard.kt`:
- Around line 128-139: The amount formatting in formatAmount currently uses
roundToInt() into an Int, which can overflow for very large transfer values.
Update the cents calculation to use a wider integer type in
RecentPayeeCard.formatAmount so the rounding fix remains safe for large amounts,
while keeping the same whole-part/decimal-part formatting behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3229857e-c9ca-4827-bd28-bf87a04d16eb
📒 Files selected for processing (1)
feature/transfer-intrabank/src/commonMain/kotlin/org/mifospay/feature/transfer/intrabank/hub/components/RecentPayeeCard.kt
Signed-off-by: bhuvan-somisetty <somisettybhuvan5@gmail.com>
Signed-off-by: bhuvan-somisetty <somisettybhuvan5@gmail.com>
8540194 to
1d3e1db
Compare
The recent payee card shows the last amount you sent to a beneficiary — and for a lot of common amounts, it was showing the wrong value. ₹5.99 appears as ₹5.98. ₹99.99 appears as ₹99.98. ₹10.30 appears as ₹10.29. In a payments app, seeing a different amount than what you actually sent is genuinely alarming.
The root cause is a classic floating-point pitfall. The old code computed the decimal part like this:
The problem:
5.99 - 5in floating-point is not0.99— it's0.9899999999999999. Multiply that by 100 and you get98.99.... Calling.toInt()truncates toward zero, so you get98instead of99. The display shows5.98.This affects any amount where the decimal part has a negative epsilon in its binary representation, which includes most
.x9endings and many others. It's unpredictable from the user's perspective, which makes it feel like data corruption.The fix avoids floating-point subtraction entirely. Instead of splitting the number and computing the decimal part through subtraction, we multiply the whole amount by 100, round to the nearest integer, and then use integer division and modulo:
Multiplying first keeps the rounding error small enough that
roundToInt()always lands on the correct cent value.Verified by tracing through the arithmetic manually for the failing cases and confirming the fixed output matches the expected display.
Summary by CodeRabbit