fix division by zero panics and input validation - #46
Open
YashejShah wants to merge 4 commits into
Open
Conversation
- Fv() panics when rate is zero, added zero-rate branch matching the pattern Pmt() already uses - Pv() same issue as Fv() - Nper() divides by rate without checking, also can produce NaN from math.Log of negative values - Flat.GetPrincipal() and GetPayment() panic when periods is 0 - Frequency.Value() returns 0 for invalid enum values, causing downstream division by zero in getInterestRatePerPeriodInDecimal - getRateRatio() newton-raphson iteration can hit zero denominators
- fixed isAlmostEqual computing ||a| - |b|| instead of |a - b|, which was masking bugs when comparing negative values - fixed sanityCheckUpdate using wrong diff formula that broke on negative rounding differences - replaced implicit 0 with paymentperiod.ENDING in test cases - added test cases for all zero-rate edge cases
- added Config.Validate() for upfront sanity checks on amount and interest before processing - added tests for validation edge cases
- bumped go.mod from 1.15 to 1.21
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.
what's wrong
was going through the code and found several division-by-zero scenarios that cause panics in production. pretty concerning for a financial calculations library.
panics fixed
Fv()panics when rate is zero — added zero-rate branch (same patternPmt()already uses)Pv()same issueNper()divides by rate without checking, also can produce NaN frommath.Logof negative valuesFlat.GetPrincipal()panics when periods is 0Frequency.Value()returns 0 for invalid enum values, causing downstream division by zerogetRateRatio()newton-raphson iteration can hit zero denominatorsother fixes
isAlmostEqualtest helper — was computing||a| - |b||instead of|a - b|, which masks bugs with negative valuessanityCheckUpdaterounding logic for negative diffsConfig.Validate()for upfront input validation0withpaymentperiod.ENDINGin testsall existing tests pass, added new ones for the edge cases.