fix: show 0 balance with decimals in classic denomination

[piotr-iohk]

Closes #342.

Description

Show 0 balance with decimals in classic denomination.
To be consistent with bitkit-ios and RN.

Preview

QA Notes

synonymdev/bitkit-e2e-tests#46

[claude]

Code Review for PR #467

Summary

This PR fixes issue #342 by ensuring that zero balances display with proper decimal places (0.00000000) in classic BTC denomination, maintaining consistency with the iOS and React Native versions of Bitkit.

---

✅ Positive Aspects

1. Correct Fix: The change properly addresses the root cause - the old pattern "###.########" would omit trailing zeros and all decimal places for zero values, while the new pattern "0.00000000" enforces all 8 decimal places.

2. Clean Implementation: The refactoring improves code clarity by:

   • Using string repetition for the pattern: "0.${"0".repeat(CLASSIC_DECIMALS)}"
   • Explicitly setting min/max fraction digits to ensure consistent formatting
   • Removing the intermediate sats variable (minor cleanup)

3. Consistency: Aligns the Android app with iOS and RN implementations, providing a unified UX across platforms.

4. Uses Constants: Properly leverages the existing CLASSIC_DECIMALS constant.

---

📋 Code Quality & Best Practices

Good:

• Follows Kotlin conventions
• Uses DecimalFormat properly with explicit configuration
• Code is readable and maintainable

Minor Suggestions:

• The pattern string construction could be extracted to a constant for reusability if this pattern is needed elsewhere
• Consider adding a KDoc comment explaining the formatting behavior for zero values

---

🐛 Potential Issues

None identified. The change is focused and correct.

---

⚡ Performance Considerations

No concerns. The change is minimal:

• String concatenation happens once per call (negligible overhead)
• Setting formatter properties is standard practice
• No performance regression expected

---

🔒 Security Concerns

None. This is a pure formatting change with no security implications.

---

🧪 Test Coverage

Observation: There are no direct unit tests for formatToClassicDisplay() or formatToModernDisplay() extension functions.

Recommendation: Consider adding unit tests to cover edge cases like zero values, 1 BTC, and fractional values.

Note: I see you have E2E tests referenced (bitkit-e2e-tests #46), which is good for catching visual regressions, but unit tests would provide faster feedback during development.

---

✨ Overall Assessment

Status: LGTM with minor suggestion

This is a clean, focused fix that properly addresses the reported issue. The implementation is correct and the code quality is good.

Recommendation: Approve and merge, with optional follow-up to add unit tests for these formatting functions.

Nice work on the fix!

[Copilot]

Pull Request Overview

This PR fixes the display of zero Bitcoin balance in classic denomination to show all 8 decimal places (e.g., "0.00000000" instead of "0"), ensuring consistency with the iOS and React Native versions of Bitkit.

• Changed the decimal format pattern from "###.########" (which hides trailing zeros) to "0.00000000" (which enforces 8 decimal places)
• Updated the formatter configuration to explicitly set minimum and maximum fraction digits to CLASSIC_DECIMALS (8)
• Removed unnecessary intermediate variable sats for cleaner code

[claude]

Pull Request Review: Fix Classic BTC Display for Zero Balance

Overview

This PR fixes issue #342 where zero balances in Classic BTC denomination were displayed as 0 instead of 0.00000000. The fix ensures consistency with bitkit-ios and the React Native version.

Code Quality ✅

Strengths:

• Clear and focused change that directly addresses the issue
• Proper use of CLASSIC_DECIMALS constant consistently
• Good code organization with explicit DecimalFormat configuration

Suggestions:

1. Pattern construction (Currency.kt:112): The dynamic pattern generation could be defined as a constant for better reusability and performance
2. Code consistency: Consider extracting format patterns as constants for maintainability

Potential Bugs/Issues ✅

No critical bugs identified, observations:

1. Locale handling: The fix properly preserves locale-based decimal separators. Tests only verify Locale.US - consider testing European locales with comma decimal separators
2. Pattern change from optional to required leading/trailing zeros is correct for the fix
3. Removed unnecessary variable assignment improves clarity

Performance Considerations ✅

Positive impact:

1. String concatenation for pattern could be pre-computed as constant
2. DecimalFormat configuration is correct and prevents unnecessary allocations
3. Minor: Creates new DecimalFormat per call - acceptable for typical usage

Security Concerns ✅

No security issues. Changes maintain type safety, no user input vulnerabilities, and use safe BigDecimal operations.

Test Coverage ⚠️

Good coverage, could be enhanced:

Existing tests:

• formatToModernDisplay uses space grouping ✅
• formatToModernDisplay handles zero ✅
• formatToClassicDisplay always shows eight decimals ✅ (directly tests the fix)
• formatToClassicDisplay converts sats to btc ✅

Recommended additions:

1. Edge cases: maximum value, negative values
2. Locale-specific tests for different decimal separators
3. Trailing zeros for whole BTC values (100000000 sats = 1.00000000)
4. Integration test for end-to-end display

Alignment with Issue #342 ✅

• Zero balance displays as 0.00000000 in Classic denomination ✅
• Consistent with bitkit-ios and React Native ✅
• Screenshot shows expected behavior ✅

Additional Observations

1. E2E Tests linked (bitkit-e2e-tests#46) - excellent
2. ULong.formatToModernDisplay exists but no ULong.formatToClassicDisplay - consider for API completeness
3. Consider adding KDoc comments explaining Modern vs Classic terminology

Summary

Recommendation: Approve with minor suggestions ✅

Well-executed fix with minimal, focused changes and appropriate test coverage. Main suggestions:

1. Extract format pattern as constant for performance/clarity
2. Add edge-case tests (negative, locales, max values)
3. Verify E2E tests pass

Great work on this fix! 🎉

[ovitrif]

👏🏻 Thanks for this nice work