Skip to content

Commit

Permalink
[LANG-1646] NumberUtils to return requested floating point type for zero
Browse files Browse the repository at this point in the history
  • Loading branch information
@aherbert
aherbert committed Mar 5, 2021
1 parent 0cfc31b commit a2b2b35
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 28 deletions.
3 changes: 2 additions & 1 deletion src/changes/changes.xml
View file
Expand Up @@ -47,7 +47,8 @@ The <action> type attribute can be add,update,fix,remove.

<release version="3.12.1" date="2021-MM-DD" description="New features and bug fixes (Java 8).">
<!-- FIX -->
<action issue="LANG-1645" type="fix" dev="aherbert" due-to="Alex Herbert">NumberUtils to recognise hex integers prefixed with +</action>
<action issue="LANG-1645" type="fix" dev="aherbert" due-to="Alex Herbert">NumberUtils.createNumber to recognise hex integers prefixed with +.</action>
<action issue="LANG-1646" type="fix" dev="aherbert" due-to="Alex Herbert">NumberUtils.createNumber to return requested floating point type for zero.</action>
</release>

<release version="3.12.0" date="2021-02-26" description="New features and bug fixes (Java 8).">
Expand Down
70 changes: 45 additions & 25 deletions src/main/java/org/apache/commons/lang3/math/NumberUtils.java
View file
Expand Up @@ -706,14 +706,17 @@ public static Number createNumber(final String str) {
// if both e and E are present, this is caught by the checks on expPos (which prevent IOOBE)
// and the parsing which will detect if e or E appear in a number due to using the wrong offset

// Detect if the return type has been requested
final boolean requestType = !Character.isDigit(lastChar) && lastChar != '.';
if (decPos > -1) { // there is a decimal point
if (expPos > -1) { // there is an exponent
if (expPos < decPos || expPos > length) { // prevents double exponent causing IOOBE
throw new NumberFormatException(str + " is not a valid number.");
}
dec = str.substring(decPos + 1, expPos);
} else {
dec = str.substring(decPos + 1);
// No exponent, but there may be a type character to remove
dec = str.substring(decPos + 1, requestType ? length - 1 : length);
}
mant = getMantissa(str, decPos);
} else {
Expand All @@ -723,19 +726,19 @@ public static Number createNumber(final String str) {
}
mant = getMantissa(str, expPos);
} else {
mant = getMantissa(str);
// No decimal, no exponent, but there may be a type character to remove
mant = getMantissa(str, requestType ? length - 1 : length);
}
dec = null;
}
if (!Character.isDigit(lastChar) && lastChar != '.') {
if (requestType) {
if (expPos > -1 && expPos < length - 1) {
exp = str.substring(expPos + 1, length - 1);
} else {
exp = null;
}
//Requesting a specific type..
final String numeric = str.substring(0, length - 1);
final boolean allZeros = isAllZeros(mant) && isAllZeros(exp);
switch (lastChar) {
case 'l' :
case 'L' :
Expand All @@ -755,7 +758,7 @@ public static Number createNumber(final String str) {
case 'F' :
try {
final Float f = createFloat(str);
if (!(f.isInfinite() || f.floatValue() == 0.0F && !allZeros)) {
if (!(f.isInfinite() || f.floatValue() == 0.0F && !isZero(mant, dec))) {
//If it's too big for a float or the float value = 0 and the string
//has non-zeros in it, then float does not have the precision we want
return f;
Expand All @@ -769,7 +772,7 @@ public static Number createNumber(final String str) {
case 'D' :
try {
final Double d = createDouble(str);
if (!(d.isInfinite() || d.doubleValue() == 0.0D && !allZeros)) {
if (!(d.isInfinite() || d.doubleValue() == 0.0D && !isZero(mant, dec))) {
return d;
}
} catch (final NumberFormatException nfe) { // NOPMD
Expand Down Expand Up @@ -809,16 +812,15 @@ public static Number createNumber(final String str) {
}

//Must be a Float, Double, BigDecimal
final boolean allZeros = isAllZeros(mant) && isAllZeros(exp);
try {
final Float f = createFloat(str);
final Double d = createDouble(str);
if (!f.isInfinite()
&& !(f.floatValue() == 0.0F && !allZeros)
&& !(f.floatValue() == 0.0F && !isZero(mant, dec))
&& f.toString().equals(d.toString())) {
return f;
}
if (!d.isInfinite() && !(d.doubleValue() == 0.0D && !allZeros)) {
if (!d.isInfinite() && !(d.doubleValue() == 0.0D && !isZero(mant, dec))) {
final BigDecimal b = createBigDecimal(str);
if (b.compareTo(BigDecimal.valueOf(d.doubleValue())) == 0) {
return d;
Expand All @@ -831,18 +833,6 @@ public static Number createNumber(final String str) {
return createBigDecimal(str);
}

/**
* <p>Utility method for {@link #createNumber(java.lang.String)}.</p>
*
* <p>Returns mantissa of the given number.</p>
*
* @param str the string representation of the number
* @return mantissa of the given number
*/
private static String getMantissa(final String str) {
return getMantissa(str, str.length());
}

/**
* <p>Utility method for {@link #createNumber(java.lang.String)}.</p>
*
Expand All @@ -860,11 +850,41 @@ private static String getMantissa(final String str, final int stopPos) {
}

/**
* <p>Utility method for {@link #createNumber(java.lang.String)}.</p>
* Utility method for {@link #createNumber(java.lang.String)}.
*
* <p>Returns {@code true} if s is {@code null}.</p>
* <p>This will check if the magnitude of the number is zero by checking if there
* are only zeros before and after the decimal place.</p>
*
* @param str the String to check
* <p>Note: It is <strong>assumed</strong> that the input string has been converted
* to either a Float or Double with a value of zero when this method is called.
* This eliminates invalid input for example {@code ".", ".D", ".e0"}.</p>
*
* <p>Thus the method only requires checking if both arguments are null, empty or
* contain only zeros.</p>
*
* <p>Given {@code s = mant + "." + dec}:</p>
* <ul>
* <li>{@code true} if s is {@code "0.0"}
* <li>{@code true} if s is {@code "0."}
* <li>{@code true} if s is {@code ".0"}
* <li>{@code false} otherwise (this assumes {@code "."} is not possible)
* </ul>
*
* @param mant the mantissa decimal digits before the decimal point (sign must be removed; never null)
* @param dec the decimal digits after the decimal point (exponent and type specifier removed;
* can be null)
* @return true if the magnitude is zero
*/
private static boolean isZero(final String mant, String dec) {
return isAllZeros(mant) && isAllZeros(dec);
}

/**
* Utility method for {@link #createNumber(java.lang.String)}.
*
* <p>Returns {@code true} if s is {@code null} or empty.</p>
*
* @param str the String to check
* @return if it is all zeros or {@code null}
*/
private static boolean isAllZeros(final String str) {
Expand All @@ -876,7 +896,7 @@ private static boolean isAllZeros(final String str) {
return false;
}
}
return !str.isEmpty();
return true;
}

//-----------------------------------------------------------------------
Expand Down
86 changes: 84 additions & 2 deletions src/test/java/org/apache/commons/lang3/math/NumberUtilsTest.java
View file
Expand Up @@ -68,11 +68,12 @@ private void compareIsCreatableWithCreateNumber(final String val, final boolean
+ " for isCreatable/createNumber using \"" + val + "\" but got " + isValid + " and " + canCreate);
}

@SuppressWarnings("deprecation")
private void compareIsNumberWithCreateNumber(final String val, final boolean expected) {
final boolean isValid = NumberUtils.isCreatable(val);
final boolean isValid = NumberUtils.isNumber(val);
final boolean canCreate = checkCreateNumber(val);
assertTrue(isValid == expected && canCreate == expected, "Expecting " + expected
+ " for isCreatable/createNumber using \"" + val + "\" but got " + isValid + " and " + canCreate);
+ " for isNumber/createNumber using \"" + val + "\" but got " + isValid + " and " + canCreate);
}

@Test
Expand Down Expand Up @@ -638,6 +639,22 @@ public void testCreateNumberMagnitude() {
// Test with +2 in final digit (+1 does not cause roll-over to BigDecimal)
assertEquals(new BigDecimal("1.7976931348623159e+308"), NumberUtils.createNumber("1.7976931348623159e+308"));

// Requested type is parsed as zero but the value is not zero
final Double nonZero1 = Double.valueOf(((double) Float.MIN_VALUE) / 2);
assertEquals(nonZero1, NumberUtils.createNumber(nonZero1.toString()));
assertEquals(nonZero1, NumberUtils.createNumber(nonZero1.toString() + "F"));
// Smallest double is 4.9e-324.
// Test a number with zero before and/or after the decimal place to hit edge cases.
final BigDecimal nonZero2 = new BigDecimal("4.9e-325");
assertEquals(nonZero2, NumberUtils.createNumber("4.9e-325"));
assertEquals(nonZero2, NumberUtils.createNumber("4.9e-325D"));
final BigDecimal nonZero3 = new BigDecimal("1e-325");
assertEquals(nonZero3, NumberUtils.createNumber("1e-325"));
assertEquals(nonZero3, NumberUtils.createNumber("1e-325D"));
final BigDecimal nonZero4 = new BigDecimal("0.1e-325");
assertEquals(nonZero4, NumberUtils.createNumber("0.1e-325"));
assertEquals(nonZero4, NumberUtils.createNumber("0.1e-325D"));

assertEquals(Integer.valueOf(0x12345678), NumberUtils.createNumber("0x12345678"));
assertEquals(Long.valueOf(0x123456789L), NumberUtils.createNumber("0x123456789"));

Expand All @@ -657,6 +674,55 @@ public void testCreateNumberMagnitude() {
assertEquals(new BigInteger("1777777777777777777777", 8), NumberUtils.createNumber("01777777777777777777777"));
}

/**
* LANG-1646: Support the requested Number type (Long, Float, Double) of valid zero input.
*/
@Test
public void testCreateNumberZero() {
// Handle integers
assertEquals(Integer.valueOf(0), NumberUtils.createNumber("0"));
assertEquals(Integer.valueOf(0), NumberUtils.createNumber("-0"));
assertEquals(Long.valueOf(0), NumberUtils.createNumber("0L"));
assertEquals(Long.valueOf(0), NumberUtils.createNumber("-0L"));

// Handle floating-point with optional leading sign, trailing exponent (eX)
// and format specifier (F or D).
// This should allow: 0. ; .0 ; 0.0 ; 0 (if exponent or format specifier is present)

// Exponent does not matter for zero
final int[] exponents = {-2345, 0, 13};
final String[] zeros = {"0.", ".0", "0.0", "0"};
final Float f0 = Float.valueOf(0);
final Float fn0 = Float.valueOf(-0F);
final Double d0 = Double.valueOf(0);
final Double dn0 = Double.valueOf(-0D);

for (final String zero : zeros) {
// Assume float if no preference.
// This requires a decimal point if there is no exponent.
if (zero.indexOf('.') != -1) {
assertCreateNumberZero(zero, f0, fn0);
}
for (final int exp : exponents) {
assertCreateNumberZero(zero + "e" + exp, f0, fn0);
}
// Type preference
assertCreateNumberZero(zero + "F", f0, fn0);
assertCreateNumberZero(zero + "D", d0, dn0);
for (final int exp : exponents) {
final String number = zero + "e" + exp;
assertCreateNumberZero(number + "F", f0, fn0);
assertCreateNumberZero(number + "D", d0, dn0);
}
}
}

private static void assertCreateNumberZero(String number, Object zero, Object negativeZero) {
assertEquals(zero, NumberUtils.createNumber(number), () -> "Input: " + number);
assertEquals(zero, NumberUtils.createNumber("+" + number), () -> "Input: +" + number);
assertEquals(negativeZero, NumberUtils.createNumber("-" + number), () -> "Input: -" + number);
}

/**
* Tests isCreatable(String) and tests that createNumber(String) returns a valid number iff isCreatable(String)
* returns false.
Expand Down Expand Up @@ -717,6 +783,14 @@ public void testIsCreatable() {
compareIsCreatableWithCreateNumber("+0xF", true); // LANG-1645
compareIsCreatableWithCreateNumber("+0xFFFFFFFF", true); // LANG-1645
compareIsCreatableWithCreateNumber("+0xFFFFFFFFFFFFFFFF", true); // LANG-1645
compareIsCreatableWithCreateNumber(".0", true); // LANG-1646
compareIsCreatableWithCreateNumber("0.", true); // LANG-1646
compareIsCreatableWithCreateNumber("0.D", true); // LANG-1646
compareIsCreatableWithCreateNumber("0e1", true); // LANG-1646
compareIsCreatableWithCreateNumber("0e1D", true); // LANG-1646
compareIsCreatableWithCreateNumber(".D", false); // LANG-1646
compareIsCreatableWithCreateNumber(".e10", false); // LANG-1646
compareIsCreatableWithCreateNumber(".e10D", false); // LANG-1646
}

@Test
Expand Down Expand Up @@ -795,6 +869,14 @@ public void testIsNumber() {
compareIsNumberWithCreateNumber("+0xF", true); // LANG-1645
compareIsNumberWithCreateNumber("+0xFFFFFFFF", true); // LANG-1645
compareIsNumberWithCreateNumber("+0xFFFFFFFFFFFFFFFF", true); // LANG-1645
compareIsNumberWithCreateNumber(".0", true); // LANG-1646
compareIsNumberWithCreateNumber("0.", true); // LANG-1646
compareIsNumberWithCreateNumber("0.D", true); // LANG-1646
compareIsNumberWithCreateNumber("0e1", true); // LANG-1646
compareIsNumberWithCreateNumber("0e1D", true); // LANG-1646
compareIsNumberWithCreateNumber(".D", false); // LANG-1646
compareIsNumberWithCreateNumber(".e10", false); // LANG-1646
compareIsNumberWithCreateNumber(".e10D", false); // LANG-1646
}

@Test
Expand Down

0 comments on commit a2b2b35

Please sign in to comment.