-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid crashing for RETDecimal.max().formated(...) #1044
Conversation
|
||
try doTest(RETDecimal.max(), expected: "3.138e39") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a new test, which fails on main
branch, but not anymore.
// `toZero` to avoid overflow. | ||
let safeBase = try! base.round( | ||
decimalPlaces: Int32(decimalPlaces), | ||
roundingMode: .toZero |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
N.B. .toNearestMidpointTowardZero
will not work either, must be .toZero
.
@@ -246,7 +246,23 @@ extension RETDecimal { | |||
let integerCount = UInt(Swift.max(digits.count - RETDecimal.scale, 1)) | |||
if integerCount > totalPlaces { | |||
let scale = RETDecimal(exponent: integerCount - totalPlaces) | |||
return (self / scale).rounded(decimalPlaces: 0) * scale | |||
let base = self / scale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that we don't want to protect ourselves against any kind of error here, only the specific case of max, no? We could just
guard self != max else {
// return the number rounded towards zero
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No because if the value is close to max we don't want to fail either. Hmm
Jira ticket: paste link here
Complements / depends on / relates to #link to other PR
Description
This PR solves / fixes / adds / includes functionality 1 and functionality 2.
Notes
Additional notes.
How to test
Happy path (or test variation 1)
Unhappy path (or test variation 2)
Screenshot
Video
paste link here
PR submission checklist