Skip to content

compare exact values in BigInteger and BigDecimal Number range checks#402

Closed
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:number-overload-compareto
Closed

compare exact values in BigInteger and BigDecimal Number range checks#402
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:number-overload-compareto

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

Carrying on from the BigInteger/BigDecimal conversion review after #400 and #401, the inherited Number-typed range overloads stood out: the type-specific overloads now compare exact values, but minValue(Number, Number), maxValue(Number, Number) and the isInRange(Number, Number, Number) that delegates to them still narrow through value.longValue()/value.doubleValue() in AbstractNumberValidator. The existing range tests only exercise the type-specific overloads, so nothing flagged it. A BigInteger past the long range truncates before the comparison (isInRange(2^64 + 50, 5, 100) wraps to 50 and is wrongly accepted), and on BigDecimalValidator a value differing from the bound only beyond double precision (2^53 + 1 versus 2^53) rounds onto the bound and is misclassified.

Override the two Number overloads in each validator to compare the exact BigInteger/BigDecimal with compareTo; isInRange inherits the correct behaviour through virtual dispatch. Keeping the fix beside the type-specific overloads means all four entry points agree, and the BigDecimal override falls back to the doubleValue() comparison for a non-finite bound so the infinity behaviour from #401 stays intact.

@garydgregory

Copy link
Copy Markdown
Member

To reduce code duplication, I'll merging #404 instead of this PR. You will be credited in changes.xml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants