Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,23 @@ public static BigIntegerValidator getInstance() {
return VALIDATOR;
}

private static BigDecimal toBigDecimal(final Number value) {
if (value instanceof BigDecimal) {
return (BigDecimal) value;
}
if (value instanceof BigInteger) {
return new BigDecimal((BigInteger) value);
}
if (value instanceof Long) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @sahvx655-wq
You missed the java.math.BigDecimal.valueOf(double) shortcut here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I've added a Double branch using BigDecimal.valueOf(((Double) value).doubleValue()) so it matches the toBigInteger helper just below, which already takes that path with the same "no need to roundtrip with a string" note. The fall-through stays on new BigDecimal(value.toString()) for the remaining types (Float keeps its own canonical short form that way rather than picking up the widened double representation).

Pushed in 3b48460 and the range tests still pass.

return BigDecimal.valueOf(value.longValue());
}
if (value instanceof Double) {
// No need to roundtrip with a string.
return BigDecimal.valueOf(((Double) value).doubleValue());
}
return new BigDecimal(value.toString());
}

private static BigInteger toBigInteger(final Object value) {
if (value instanceof Long) {
return BigInteger.valueOf(((Long) value).longValue());
Expand Down Expand Up @@ -163,7 +180,8 @@ public boolean maxValue(final BigInteger value, final long max) {
*
* <p>
* This overrides the {@link Number} overload inherited from the superclass, which narrows the value to a {@code long} before comparing and so loses
* magnitude for a {@code BigInteger} outside the long range.
* magnitude for a {@code BigInteger} outside the long range. The operands are compared as {@code BigDecimal} so a non-integer bound keeps its fractional
* part instead of being truncated towards zero.
* </p>
*
* @param value The value validation is being performed on.
Expand All @@ -172,7 +190,7 @@ public boolean maxValue(final BigInteger value, final long max) {
*/
@Override
public boolean maxValue(final Number value, final Number max) {
return toBigInteger(value).compareTo(toBigInteger(max)) <= 0;
return toBigDecimal(value).compareTo(toBigDecimal(max)) <= 0;
}

/**
Expand All @@ -191,7 +209,8 @@ public boolean minValue(final BigInteger value, final long min) {
*
* <p>
* This overrides the {@link Number} overload inherited from the superclass, which narrows the value to a {@code long} before comparing and so loses
* magnitude for a {@code BigInteger} outside the long range.
* magnitude for a {@code BigInteger} outside the long range. The operands are compared as {@code BigDecimal} so a non-integer bound keeps its fractional
* part instead of being truncated towards zero.
* </p>
*
* @param value The value validation is being performed on.
Expand All @@ -200,7 +219,7 @@ public boolean minValue(final BigInteger value, final long min) {
*/
@Override
public boolean minValue(final Number value, final Number min) {
return toBigInteger(value).compareTo(toBigInteger(min)) >= 0;
return toBigDecimal(value).compareTo(toBigDecimal(min)) >= 0;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Locale;

Expand Down Expand Up @@ -214,4 +215,27 @@ void testNumberRangeOutsideLongRange() {
assertEquals(50L, wrapsIntoRange.longValue());
assertFalse(instance.isInRange(wrapsIntoRange, min, max));
}

/**
* The {@link Number} overloads must compare against the exact bound. A non-integer bound was converted with BigInteger.toBigInteger(), which truncates
* towards zero, so a fractional minimum was floored (wrongly admitting a value below it) and a fractional maximum was floored too (wrongly admitting a value
* above a negative maximum).
*/
@Test
void testNumberRangeFractionalBound() {
final AbstractNumberValidator instance = BigIntegerValidator.getInstance();
final Number five = BigInteger.valueOf(5);
// 5 >= 5.9 is false, but flooring the bound to 5 made it pass
assertFalse(instance.minValue(five, Double.valueOf(5.9)));
assertFalse(instance.minValue(five, new BigDecimal("5.9")));
// a whole-number bound is unaffected
assertTrue(instance.minValue(five, BigInteger.valueOf(5)));

final Number minusFive = BigInteger.valueOf(-5);
// -5 <= -5.9 is false, but flooring the bound to -5 made it pass
assertFalse(instance.maxValue(minusFive, Double.valueOf(-5.9)));
assertFalse(instance.maxValue(minusFive, new BigDecimal("-5.9")));
// -6 <= -5.9 is true
assertTrue(instance.maxValue(BigInteger.valueOf(-6), Double.valueOf(-5.9)));
}
}
Loading