Break MaxMagnitude and MinMagnitude ties by sign, as int and double do - #113
Merged
Merged
Conversation
…o [patch] MaxMagnitude and MinMagnitude returned the first argument whenever the two magnitudes were equal, so MaxMagnitude(-2, 2) gave -2 and MinMagnitude(2, -2) gave 2, and the answer depended on argument order. INumberBase, as int, double and decimal implement it, returns the positive value from MaxMagnitude and the negative value from MinMagnitude on a tie. MaxMagnitudeNumber and MinMagnitudeNumber delegate to them and inherit the fix. The existing MinMagnitude tests asserted the old behaviour with (1, -1); they now expect -1. New tests check both argument orders of the tie against int's own MaxMagnitude and MinMagnitude, and the non-tie cases for each sign. Fixes #111 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QK96a24CjVhUK9u1Ss2YNN
This was referenced Sep 26, 2026
… if [patch] SonarCloud S3358 flagged the nested conditional expressions. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QK96a24CjVhUK9u1Ss2YNN
|
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
2 participants
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.
Fixes #111
Summary
MaxMagnitudeandMinMagnitudereturned the first argument whenever the magnitudes were equal, soMaxMagnitude(-2, 2)gave-2andMinMagnitude(2, -2)gave2. The result also changed with argument order. TheINumberBase<T>contract, asint,double, anddecimalimplement it, returns the positive value fromMaxMagnitudeand the negative value fromMinMagnitudeon a tie. Both methods now compare magnitudes once and break a tie by sign.MaxMagnitudeNumberandMinMagnitudeNumberdelegate to them, so they pick up the fix too.Tests
TestStaticMinMagnitudeandTestStaticMinMagnitudeNumbertests expected the old answer for(1, -1). They now expect-1.TestMagnitudeTiesFollowIntruns(-2, 2)and(2, -2)through all four methods and compares each result withint's own answer.TestMagnitudeWithoutATiePicksByAbsoluteValuecovers the non-tie cases for each sign and argument order.🤖 Generated with Claude Code
https://claude.ai/code/session_01QK96a24CjVhUK9u1Ss2YNN
Generated by Claude Code