ICU-13443 Making MAX_INT_FRAC_SIG checking consistent between inclusive and exclusive...

X-SVN-Rev: 40746

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/FractionRounder.java b/icu4j/main/classes/core/src/com/ibm/icu/number/FractionRounder.java
index 5662e360b82fcd42012e73bd8ab3af466b355441..2ced2a44bad4d3305597a0f0045ef903439ac7c7 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/number/FractionRounder.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/number/FractionRounder.java
@@ -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)");
         }
     }
 }
\ No newline at end of file

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/IntegerWidth.java b/icu4j/main/classes/core/src/com/ibm/icu/number/IntegerWidth.java
index 90fe0dcc391371aa166eec71bd72aec8dff60864..4d75894fad3f970da0b2370e4aae11e801d6d603 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/number/IntegerWidth.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/number/IntegerWidth.java
@@ -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)");
         }
     }
 }
\ No newline at end of file

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/Rounder.java b/icu4j/main/classes/core/src/com/ibm/icu/number/Rounder.java
index 5624fa8a5f802dc92a848e63957a5fdbf345c4de..38db8c861fba0b23e6bce6b217289cacc76f201e 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/number/Rounder.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/number/Rounder.java
@@ -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)");
         }
     }
 

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/ScientificNotation.java b/icu4j/main/classes/core/src/com/ibm/icu/number/ScientificNotation.java
index fe0e274d26b0f7e8a0370e5ec9bd8e06b74155e6..c0f7a60293af1d9b9a973a884453a072d7964f83 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/number/ScientificNotation.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/number/ScientificNotation.java
@@ -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)");
         }
     }
 

diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
index 835a5fa320ad87060407591e3d8d1d11ab2d5557..af6df82eeac5295ea452a521eae4efa7a10568f0 100644 (file)
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
@@ -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,