ICU-13443 Making MAX_INT_FRAC_SIG checking consistent between inclusive and exclusive ranges. (Changing all comparisons to be inclusive.)

X-SVN-Rev: 40746
This commit is contained in:
Shane Carr 2017-12-20 01:41:08 +00:00
parent 76a9c82e1e
commit 73569e99bd
5 changed files with 142 additions and 26 deletions

View file

@ -39,11 +39,11 @@ public abstract class FractionRounder extends Rounder {
* @see NumberFormatter
*/
public Rounder withMinDigits(int minSignificantDigits) {
if (minSignificantDigits > 0 && minSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
if (minSignificantDigits >= 1 && minSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
return constructFractionSignificant(this, minSignificantDigits, -1);
} else {
throw new IllegalArgumentException(
"Significant digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Significant digits must be between 1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -67,11 +67,11 @@ public abstract class FractionRounder extends Rounder {
* @see NumberFormatter
*/
public Rounder withMaxDigits(int maxSignificantDigits) {
if (maxSignificantDigits > 0 && maxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
if (maxSignificantDigits >= 1 && maxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
return constructFractionSignificant(this, -1, maxSignificantDigits);
} else {
throw new IllegalArgumentException(
"Significant digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Significant digits must be between 1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
}

View file

@ -42,11 +42,11 @@ public class IntegerWidth {
public static IntegerWidth zeroFillTo(int minInt) {
if (minInt == 1) {
return DEFAULT;
} else if (minInt >= 0 && minInt < RoundingUtils.MAX_INT_FRAC_SIG) {
} else if (minInt >= 0 && minInt <= RoundingUtils.MAX_INT_FRAC_SIG) {
return new IntegerWidth(minInt, -1);
} else {
throw new IllegalArgumentException(
"Integer digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Integer digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -56,7 +56,7 @@ public class IntegerWidth {
* For example, with maxInt=3, the number 1234 will get printed as "234".
*
* @param maxInt
* The maximum number of places before the decimal separator.
* The maximum number of places before the decimal separator. maxInt == -1 means no truncation.
* @return An IntegerWidth for passing to the NumberFormatter integerWidth() setter.
* @draft ICU 60
* @provisional This API might change or be removed in a future release.
@ -65,13 +65,13 @@ public class IntegerWidth {
public IntegerWidth truncateAt(int maxInt) {
if (maxInt == this.maxInt) {
return this;
} else if (maxInt >= 0 && maxInt < RoundingUtils.MAX_INT_FRAC_SIG) {
} else if (maxInt >= 0 && maxInt <= RoundingUtils.MAX_INT_FRAC_SIG && maxInt >= minInt) {
return new IntegerWidth(minInt, maxInt);
} else if (maxInt == -1) {
return new IntegerWidth(minInt, maxInt);
return new IntegerWidth(minInt, -1);
} else {
throw new IllegalArgumentException(
"Integer digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Integer digits must be between -1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
}

View file

@ -98,7 +98,7 @@ public abstract class Rounder implements Cloneable {
return constructFraction(minMaxFractionPlaces, minMaxFractionPlaces);
} else {
throw new IllegalArgumentException(
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -118,11 +118,11 @@ public abstract class Rounder implements Cloneable {
* @see NumberFormatter
*/
public static FractionRounder minFraction(int minFractionPlaces) {
if (minFractionPlaces >= 0 && minFractionPlaces < RoundingUtils.MAX_INT_FRAC_SIG) {
if (minFractionPlaces >= 0 && minFractionPlaces <= RoundingUtils.MAX_INT_FRAC_SIG) {
return constructFraction(minFractionPlaces, -1);
} else {
throw new IllegalArgumentException(
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -139,11 +139,11 @@ public abstract class Rounder implements Cloneable {
* @see NumberFormatter
*/
public static FractionRounder maxFraction(int maxFractionPlaces) {
if (maxFractionPlaces >= 0 && maxFractionPlaces < RoundingUtils.MAX_INT_FRAC_SIG) {
if (maxFractionPlaces >= 0 && maxFractionPlaces <= RoundingUtils.MAX_INT_FRAC_SIG) {
return constructFraction(0, maxFractionPlaces);
} else {
throw new IllegalArgumentException(
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -168,7 +168,7 @@ public abstract class Rounder implements Cloneable {
return constructFraction(minFractionPlaces, maxFractionPlaces);
} else {
throw new IllegalArgumentException(
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Fraction length must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -188,11 +188,11 @@ public abstract class Rounder implements Cloneable {
* @see NumberFormatter
*/
public static Rounder fixedDigits(int minMaxSignificantDigits) {
if (minMaxSignificantDigits > 0 && minMaxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
if (minMaxSignificantDigits >= 1 && minMaxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
return constructSignificant(minMaxSignificantDigits, minMaxSignificantDigits);
} else {
throw new IllegalArgumentException(
"Significant digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Significant digits must be between 1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -211,11 +211,11 @@ public abstract class Rounder implements Cloneable {
* @see NumberFormatter
*/
public static Rounder minDigits(int minSignificantDigits) {
if (minSignificantDigits > 0 && minSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
if (minSignificantDigits >= 1 && minSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
return constructSignificant(minSignificantDigits, -1);
} else {
throw new IllegalArgumentException(
"Significant digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Significant digits must be between 1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -230,11 +230,11 @@ public abstract class Rounder implements Cloneable {
* @see NumberFormatter
*/
public static Rounder maxDigits(int maxSignificantDigits) {
if (maxSignificantDigits > 0 && maxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
if (maxSignificantDigits >= 1 && maxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
return constructSignificant(0, maxSignificantDigits);
} else {
throw new IllegalArgumentException(
"Significant digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Significant digits must be between 1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}
@ -252,12 +252,12 @@ public abstract class Rounder implements Cloneable {
* @see NumberFormatter
*/
public static Rounder minMaxDigits(int minSignificantDigits, int maxSignificantDigits) {
if (minSignificantDigits > 0 && maxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG
if (minSignificantDigits >= 1 && maxSignificantDigits <= RoundingUtils.MAX_INT_FRAC_SIG
&& minSignificantDigits <= maxSignificantDigits) {
return constructSignificant(minSignificantDigits, maxSignificantDigits);
} else {
throw new IllegalArgumentException(
"Significant digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Significant digits must be between 1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}

View file

@ -55,13 +55,13 @@ public class ScientificNotation extends Notation implements Cloneable {
* @see NumberFormatter
*/
public ScientificNotation withMinExponentDigits(int minExponentDigits) {
if (minExponentDigits >= 0 && minExponentDigits < RoundingUtils.MAX_INT_FRAC_SIG) {
if (minExponentDigits >= 1 && minExponentDigits <= RoundingUtils.MAX_INT_FRAC_SIG) {
ScientificNotation other = (ScientificNotation) this.clone();
other.minExponentDigits = minExponentDigits;
return other;
} else {
throw new IllegalArgumentException(
"Integer digits must be between 0 and " + RoundingUtils.MAX_INT_FRAC_SIG);
"Integer digits must be between 1 and " + RoundingUtils.MAX_INT_FRAC_SIG + " (inclusive)");
}
}

View file

@ -4,10 +4,17 @@ package com.ibm.icu.dev.test.number;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import org.junit.Ignore;
import org.junit.Test;
@ -16,6 +23,7 @@ import com.ibm.icu.impl.number.Padder;
import com.ibm.icu.impl.number.Padder.PadPosition;
import com.ibm.icu.impl.number.PatternStringParser;
import com.ibm.icu.number.FormattedNumber;
import com.ibm.icu.number.FractionRounder;
import com.ibm.icu.number.Grouper;
import com.ibm.icu.number.IntegerWidth;
import com.ibm.icu.number.LocalizedNumberFormatter;
@ -25,6 +33,7 @@ import com.ibm.icu.number.NumberFormatter.DecimalSeparatorDisplay;
import com.ibm.icu.number.NumberFormatter.SignDisplay;
import com.ibm.icu.number.NumberFormatter.UnitWidth;
import com.ibm.icu.number.Rounder;
import com.ibm.icu.number.ScientificNotation;
import com.ibm.icu.number.UnlocalizedNumberFormatter;
import com.ibm.icu.text.DecimalFormatSymbols;
import com.ibm.icu.text.NumberingSystem;
@ -1560,6 +1569,113 @@ public class NumberFormatterApiTest {
"1.00 US dollars");
}
@Test
public void validRanges() throws NoSuchMethodException, IllegalAccessException {
Method[] methodsWithOneArgument = new Method[] { Rounder.class.getDeclaredMethod("fixedFraction", Integer.TYPE),
Rounder.class.getDeclaredMethod("minFraction", Integer.TYPE),
Rounder.class.getDeclaredMethod("maxFraction", Integer.TYPE),
Rounder.class.getDeclaredMethod("fixedDigits", Integer.TYPE),
Rounder.class.getDeclaredMethod("minDigits", Integer.TYPE),
Rounder.class.getDeclaredMethod("maxDigits", Integer.TYPE),
FractionRounder.class.getDeclaredMethod("withMinDigits", Integer.TYPE),
FractionRounder.class.getDeclaredMethod("withMaxDigits", Integer.TYPE),
ScientificNotation.class.getDeclaredMethod("withMinExponentDigits", Integer.TYPE),
IntegerWidth.class.getDeclaredMethod("zeroFillTo", Integer.TYPE),
IntegerWidth.class.getDeclaredMethod("truncateAt", Integer.TYPE), };
Method[] methodsWithTwoArguments = new Method[] {
Rounder.class.getDeclaredMethod("minMaxFraction", Integer.TYPE, Integer.TYPE),
Rounder.class.getDeclaredMethod("minMaxDigits", Integer.TYPE, Integer.TYPE), };
final int EXPECTED_MAX_INT_FRAC_SIG = 100;
final String expectedSubstring0 = "between 0 and 100 (inclusive)";
final String expectedSubstring1 = "between 1 and 100 (inclusive)";
final String expectedSubstringN1 = "between -1 and 100 (inclusive)";
// We require that the upper bounds all be 100 inclusive.
// The lower bound may be either -1, 0, or 1.
Set<String> methodsWithLowerBound1 = new HashSet();
methodsWithLowerBound1.add("fixedDigits");
methodsWithLowerBound1.add("minDigits");
methodsWithLowerBound1.add("maxDigits");
methodsWithLowerBound1.add("minMaxDigits");
methodsWithLowerBound1.add("withMinDigits");
methodsWithLowerBound1.add("withMaxDigits");
methodsWithLowerBound1.add("withMinExponentDigits");
Set<String> methodsWithLowerBoundN1 = new HashSet();
methodsWithLowerBoundN1.add("truncateAt");
// Some of the methods require an object to be called upon.
Map<String, Object> targets = new HashMap<String, Object>();
targets.put("withMinDigits", Rounder.integer());
targets.put("withMaxDigits", Rounder.integer());
targets.put("withMinExponentDigits", Notation.scientific());
targets.put("truncateAt", IntegerWidth.zeroFillTo(0));
for (int argument = -2; argument <= EXPECTED_MAX_INT_FRAC_SIG + 2; argument++) {
for (Method method : methodsWithOneArgument) {
String message = "i = " + argument + "; method = " + method.getName();
int lowerBound = methodsWithLowerBound1.contains(method.getName()) ? 1
: methodsWithLowerBoundN1.contains(method.getName()) ? -1 : 0;
String expectedSubstring = lowerBound == 0 ? expectedSubstring0
: lowerBound == 1 ? expectedSubstring1 : expectedSubstringN1;
Object target = targets.get(method.getName());
try {
method.invoke(target, argument);
assertTrue(message, argument >= lowerBound && argument <= EXPECTED_MAX_INT_FRAC_SIG);
} catch (InvocationTargetException e) {
assertTrue(message, argument < lowerBound || argument > EXPECTED_MAX_INT_FRAC_SIG);
// Ensure the exception message contains the expected substring
String actualMessage = e.getCause().getMessage();
assertNotEquals(message + ": " + actualMessage, -1, actualMessage.indexOf(expectedSubstring));
}
}
for (Method method : methodsWithTwoArguments) {
String message = "i = " + argument + "; method = " + method.getName();
int lowerBound = methodsWithLowerBound1.contains(method.getName()) ? 1
: methodsWithLowerBoundN1.contains(method.getName()) ? -1 : 0;
String expectedSubstring = lowerBound == 0 ? expectedSubstring0 : expectedSubstring1;
Object target = targets.get(method.getName());
// Check range on the first argument
try {
// Pass EXPECTED_MAX_INT_FRAC_SIG as the second argument so arg1 <= arg2 in expected cases
method.invoke(target, argument, EXPECTED_MAX_INT_FRAC_SIG);
assertTrue(message, argument >= lowerBound && argument <= EXPECTED_MAX_INT_FRAC_SIG);
} catch (InvocationTargetException e) {
assertTrue(message, argument < lowerBound || argument > EXPECTED_MAX_INT_FRAC_SIG);
// Ensure the exception message contains the expected substring
String actualMessage = e.getCause().getMessage();
assertNotEquals(message + ": " + actualMessage, -1, actualMessage.indexOf(expectedSubstring));
}
// Check range on the second argument
try {
// Pass lowerBound as the first argument so arg1 <= arg2 in expected cases
method.invoke(target, lowerBound, argument);
assertTrue(message, argument >= lowerBound && argument <= EXPECTED_MAX_INT_FRAC_SIG);
} catch (InvocationTargetException e) {
assertTrue(message, argument < lowerBound || argument > EXPECTED_MAX_INT_FRAC_SIG);
// Ensure the exception message contains the expected substring
String actualMessage = e.getCause().getMessage();
assertNotEquals(message + ": " + actualMessage, -1, actualMessage.indexOf(expectedSubstring));
}
// Check that first argument must be less than or equal to second argument
try {
method.invoke(target, argument, argument - 1);
org.junit.Assert.fail();
} catch (InvocationTargetException e) {
// Pass
}
}
}
// Check first argument less than or equal to second argument on IntegerWidth
try {
IntegerWidth.zeroFillTo(4).truncateAt(2);
org.junit.Assert.fail();
} catch (IllegalArgumentException e) {
// Pass
}
}
private static void assertFormatDescending(
String message,
String skeleton,