refactor(core): name the round-to-display-precision step on FiatAmount - #702
Merged
Conversation
Nine call sites rounded a fiat value to its currency's fraction digits by hand, each writing out `value.rounded(to: currency.maximumFractionDigits)` and rebuilding a FiatAmount around the result. They are all doing the same thing — taking the figure a user is actually shown, so a comparison against a displayed bound holds — and that concept had no name. `roundedToSmallestUnit()` gives it one, next to the existing `flooredToSmallestUnit()`. The pair now reads as the choice it is: round half-up for a figure being compared, floor for a ceiling that rounding up would break. Both are named in each other's docs so the wrong one is harder to reach for. Behaviour is unchanged; the helper is the same expression. Two call sites also drop a hand-unpacked `.value` comparison in favour of comparing the FiatAmounts themselves, which is safe where a preceding guard establishes the currencies match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #701, which added the ninth copy of this idiom.
Nine call sites rounded a fiat value to its currency's fraction digits by hand:
They are all reaching for the same thing — the figure a user is actually shown, so a comparison against a displayed bound holds ("what we display is what we accept"). The concept had no name, which is how it ended up written out nine times.
FiatAmount.roundedToSmallestUnit()names it, next to the existingflooredToSmallestUnit(). The two now read as the choice they are: round half-up for a figure being compared, floor for a ceiling that rounding up would break, such as a spend limit derived from a balance. Each doc points at the other so the wrong one is harder to reach for by accident.Call sites routed through it:
FiatAmount.converted(to:rates:)TipFloor.isMet(by:)UserFlags.TipPresets.meetsMinimum(_:)AddMoneyGate.canPayLaunchCostStoredBalance.displayedUSDFEnterAmountCalculator.isWithinDisplayLimitWithdrawViewModel.displayFeeCoinbaseDepositOperation.checkMinimumBuyAmountViewModel.computePaymentAmountBehaviour is unchanged — the helper is the same expression. Two of the call sites additionally drop a hand-unpacked
.valuecomparison in favour of comparing theFiatAmounts directly; that is safe in both because a preceding guard establishes the currencies match, which is whatFiatAmount: Comparablepreconditions on.SendTipSheet'samount == amount.rounded(to: 0)is left alone: it is a whole-number check on a bareDecimal, not this concept.Five tests cover the new helper in
FiatAmountTests— half-up above the half-unit, truncation below it, an exact value untouched, a zero-decimal currency (JPY), and the wallet's case of two values that display alike collapsing to the same figure.