Skip to content

Use compareTo() in BigIntegerValidator.minValue()#400

Merged
garydgregory merged 1 commit into
apache:masterfrom
sahvx655-wq:bigint-minvalue-compareto
Jun 19, 2026
Merged

Use compareTo() in BigIntegerValidator.minValue()#400
garydgregory merged 1 commit into
apache:masterfrom
sahvx655-wq:bigint-minvalue-compareto

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

minValue narrows the BigInteger to a long before comparing, so any value outside long range gives the wrong answer, whereas the sibling maxValue and isInRange already compare with compareTo.

  1. value.longValue() keeps only the low 64 bits, so a value such as 2^63 is compared as 0 and a value just past Long.MIN_VALUE wraps to a large positive, which makes minValue disagree with this class's own maxValue/isInRange.
  2. commit eafeda9 moved isInRange and maxValue onto compareTo(BigInteger.valueOf(...)) but left minValue on longValue(), so the gap stayed: minValue(2^64, 5) returns false although 2^64 is clearly greater than 5.

I switched minValue to the same compareTo form. Two assertions in testBigIntegerAboveLongMaxValue and testBigIntegerBelowLongMinValue were matching the truncated output (a value above Long.MAX_VALUE was treated as not greater-or-equal to Long.MAX_VALUE), so I corrected those to the right results and added a small test built from exact BigIntegers, which sidesteps the double rounding the parsed cases go through.

@garydgregory garydgregory changed the title use compareTo in BigIntegerValidator.minValue Use compareTo in BigIntegerValidator.minValue Jun 19, 2026
@garydgregory garydgregory changed the title Use compareTo in BigIntegerValidator.minValue Use compareTo() in BigIntegerValidator.minValue() Jun 19, 2026
@garydgregory garydgregory merged commit fd569a5 into apache:master Jun 19, 2026
10 checks passed
@garydgregory

Copy link
Copy Markdown
Member

Merged 🚀 Thank you @sahvx655-wq
Please review other BigInteger and BigDecimal to primitive conversion issues if you can.
TY!

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