ICU-23264 Add validateAndGet function to MeasureUnit for efficient unit validation

See unicode-org#3780

Author: younies

icu4c/source/i18n/measunit.cpp

@@ -2546,6 +2546,14 @@ MeasureUnit MeasureUnit::getToJp() {
 
 // End generated code for measunit.cpp
 
+/**
+ * Generic binary search function for string arrays.
+ * @param array the array of strings to search in
+ * @param start the starting index (inclusive)
+ * @param end the ending index (exclusive)
+ * @param key the string to search for
+ * @return the index if found, -1 if not found
+ */
 static int32_t binarySearch(
         const char * const * array, int32_t start, int32_t end, StringPiece key) {
     while (start < end) {
@@ -2563,6 +2571,33 @@ static int32_t binarySearch(
     return -1;
 }
 
+/**
+ * Helper function to get the subtype range for a given type index.
+ * @param typeIdx the type index (0 to UPRV_LENGTHOF(gTypes)-1)
+ * @param start will be set to the starting index in gSubTypes
+ * @param end will be set to the ending index in gSubTypes (exclusive)
+ * @return the length of the range (end - start)
+ */
+static int32_t getSubtypeRange(int32_t typeIdx, int32_t &start, int32_t &end) {
+    U_ASSERT(typeIdx >= 0 && typeIdx < UPRV_LENGTHOF(gTypes));
+    start = gOffsets[typeIdx];
+    end = gOffsets[typeIdx + 1];
+    return end - start;
+}
+
+/**
+ * Binary search function that searches for a subtype in gSubTypes within the range
+ * corresponding to the given type.
+ * @param typeIdx the type index (0 to UPRV_LENGTHOF(gTypes)-1)
+ * @param key the subtype string to search for
+ * @return the index in gSubTypes if found, -1 if not found
+ */
+static int32_t binarySearch(int32_t typeIdx, StringPiece key) {
+    int32_t start, end;
+    (void)getSubtypeRange(typeIdx, start, end);
+    return binarySearch(gSubTypes, start, end, key);
+}
+
 MeasureUnit::MeasureUnit() : MeasureUnit(kBaseTypeIdx, kBaseSubTypeIdx) {
 }
 
@@ -2680,7 +2715,8 @@ int32_t MeasureUnit::getAvailable(
     }
     int32_t idx = 0;
     for (int32_t typeIdx = 0; typeIdx < UPRV_LENGTHOF(gTypes); ++typeIdx) {
-        int32_t len = gOffsets[typeIdx + 1] - gOffsets[typeIdx];
+        int32_t start, end;
+        int32_t len = getSubtypeRange(typeIdx, start, end);
         for (int32_t subTypeIdx = 0; subTypeIdx < len; ++subTypeIdx) {
             dest[idx].setTo(typeIdx, subTypeIdx);
             ++idx;
@@ -2702,7 +2738,8 @@ int32_t MeasureUnit::getAvailable(
     if (typeIdx == -1) {
         return 0;
     }
-    int32_t len = gOffsets[typeIdx + 1] - gOffsets[typeIdx];
+    int32_t start, end;
+    int32_t len = getSubtypeRange(typeIdx, start, end);
     if (destCapacity < len) {
         errorCode = U_BUFFER_OVERFLOW_ERROR;
         return len;
@@ -2729,6 +2766,24 @@ StringEnumeration* MeasureUnit::getAvailableTypes(UErrorCode &errorCode) {
     return result;
 }
 
+bool MeasureUnit::validateAndGet(StringPiece type, StringPiece subtype, MeasureUnit &result) {
+    // Find the type index using binary search
+    int32_t typeIdx = binarySearch(gTypes, 0, UPRV_LENGTHOF(gTypes), type);
+    if (typeIdx == -1) {
+        return false;  // Type not found
+    }
+    
+    // Find the subtype within the type's range using binary search
+    int32_t subtypeIdx = binarySearch(typeIdx, subtype);
+    if (subtypeIdx == -1) {
+        return false;  // Subtype not found
+    }
+    
+    // Create the MeasureUnit and return it
+    result.setTo(typeIdx, subtypeIdx - gOffsets[typeIdx]);
+    return true;
+}
+
 bool MeasureUnit::findBySubType(StringPiece subType, MeasureUnit* output) {
     // Sanity checking kCurrencyOffset and final entry in gOffsets
     U_ASSERT(uprv_strcmp(gTypes[kCurrencyOffset], "currency") == 0);
@@ -2739,7 +2794,7 @@ bool MeasureUnit::findBySubType(StringPiece subType, MeasureUnit* output) {
         if (t == kCurrencyOffset) {
             continue;
         }
-        int32_t st = binarySearch(gSubTypes, gOffsets[t], gOffsets[t + 1], subType);
+        int32_t st = binarySearch(t, subType);
         if (st >= 0) {
             output->setTo(t, st - gOffsets[t]);
             return true;
@@ -2763,7 +2818,7 @@ void MeasureUnit::initTime(const char *timeId) {
     int32_t result = binarySearch(gTypes, 0, UPRV_LENGTHOF(gTypes), "duration");
     U_ASSERT(result != -1);
     fTypeId = result;
-    result = binarySearch(gSubTypes, gOffsets[fTypeId], gOffsets[fTypeId + 1], timeId);
+    result = binarySearch(fTypeId, timeId);
     U_ASSERT(result != -1);
     fSubTypeId = result - gOffsets[fTypeId];
 }
@@ -2772,8 +2827,7 @@ void MeasureUnit::initCurrency(StringPiece isoCurrency) {
     int32_t result = binarySearch(gTypes, 0, UPRV_LENGTHOF(gTypes), "currency");
     U_ASSERT(result != -1);
     fTypeId = result;
-    result = binarySearch(
-            gSubTypes, gOffsets[fTypeId], gOffsets[fTypeId + 1], isoCurrency);
+    result = binarySearch(fTypeId, isoCurrency);
     if (result == -1) {
         UErrorCode status = U_ZERO_ERROR;
         fImpl = new MeasureUnitImpl(MeasureUnitImpl::forCurrencyCode(isoCurrency, status));

icu4c/source/i18n/number_skeletons.cpp

@@ -1072,25 +1072,12 @@ void blueprint_helpers::parseMeasureUnitOption(const StringSegment& segment, Mac
     CharString subType;
     SKELETON_UCHAR_TO_CHAR(subType, stemString, firstHyphen + 1, stemString.length(), status);
 
-    // Note: the largest type as of this writing (Aug 2020) is "volume", which has 33 units.
-    static constexpr int32_t CAPACITY = 40;
-    MeasureUnit units[CAPACITY];
-    UErrorCode localStatus = U_ZERO_ERROR;
-    int32_t numUnits = MeasureUnit::getAvailable(type.data(), units, CAPACITY, localStatus);
-    if (U_FAILURE(localStatus)) {
-        // More than 30 units in this type?
-        status = U_INTERNAL_PROGRAM_ERROR;
+    MeasureUnit unit;
+    if (MeasureUnit::validateAndGet(type.toStringPiece(), subType.toStringPiece(), unit)) {
+        macros.unit = unit;
         return;
-    }
-    for (int32_t i = 0; i < numUnits; i++) {
-        auto& unit = units[i];
-        if (uprv_strcmp(subType.data(), unit.getSubtype()) == 0) {
-            macros.unit = unit;
-            return;
-        }
-    }
-
-    // throw new SkeletonSyntaxException("Unknown measure unit", segment);
+    } 
+    
     status = U_NUMBER_SKELETON_SYNTAX_ERROR;
 }
 

icu4c/source/i18n/unicode/measunit.h

@@ -720,6 +720,22 @@ class U_I18N_API MeasureUnit: public UObject {
      */
     static StringEnumeration* getAvailableTypes(UErrorCode &errorCode);
 
+#ifndef U_HIDE_INTERNAL_API
+    /**
+     * Validates that a specific type and subtype combination exists and retrieve the unit.
+     * 
+     * <p> Note: This is more efficient than calling getAvailable() when you only need
+     * to validate and retrieve a single unit.
+     *
+     * @param type the unit type (e.g., "length", "mass", "volume")
+     * @param subtype the unit subtype (e.g., "meter", "kilogram", "liter")
+     * @param result if the unit is valid, this will be set to the MeasureUnit
+     * @return true if the type/subtype combination is valid, false otherwise
+     * @internal
+     */
+    static bool validateAndGet(StringPiece type, StringPiece subtype, MeasureUnit &result);
+#endif /* U_HIDE_INTERNAL_API */
+
     /**
      * Return the class ID for this class. This is useful only for comparing to
      * a return value from getDynamicClassID(). For example:

icu4c/source/test/intltest/numbertest_api.cpp

@@ -888,6 +888,17 @@ void NumberFormatterApiTest::unitMeasure() {
             Locale("en"),
             100,
             u"100");
+    
+      // This test checks the behavior of using a fixed-size length for the units array with a fixed number of units.
+      // The array size is fixed, and we are now using a binary search approach.
+      assertFormatSingle(
+              u"One of the latest unit, volume-teaspoon",
+                    u"measure-unit/volume-teaspoon",
+                    u"unit/teaspoon",
+                    NumberFormatter::with().unit(MeasureUnit::forIdentifier("teaspoon", status)),
+                    Locale("en-US"),
+                    100,
+                    u"100 tsp");
 
     // TODO: desired behaviour for this "pathological" case?
     // Since this is pointless, we don't test that its behaviour doesn't change.

icu4j/main/core/src/main/java/com/ibm/icu/number/NumberSkeletonImpl.java

@@ -26,7 +26,6 @@
 import com.ibm.icu.util.StringTrieBuilder;
 import java.math.BigDecimal;
 import java.math.RoundingMode;
-import java.util.Set;
 
 /**
  * @author sffc
@@ -1095,12 +1094,10 @@ private static void parseMeasureUnitOption(StringSegment segment, MacroProps mac
             }
             String type = segment.subSequence(0, firstHyphen).toString();
             String subType = segment.subSequence(firstHyphen + 1, segment.length()).toString();
-            Set<MeasureUnit> units = MeasureUnit.getAvailable(type);
-            for (MeasureUnit unit : units) {
-                if (subType.equals(unit.getSubtype())) {
-                    macros.unit = unit;
-                    return;
-                }
+            MeasureUnit unit = MeasureUnit.getUnit(type, subType);
+            if (unit != null) {
+                macros.unit = unit;
+                return;
             }
             throw new SkeletonSyntaxException("Unknown measure unit", segment);
         }

icu4j/main/core/src/main/java/com/ibm/icu/util/MeasureUnit.java

@@ -870,6 +870,26 @@ public static MeasureUnit findBySubType(String subType) {
         return null;
     }
 
+    /**
+     * Returns the MeasureUnit instance if the given type and subtype combination is valid, or null
+     * otherwise.
+     *
+     * <p>Example: "length", "meter" -> METER "length", "kilometer" -> KILOMETER "length",
+     * "kilometer-per-hour" -> null --> not valid
+     *
+     * @param type the unit type (e.g., "length", "mass", "volume")
+     * @param subtype the unit subtype (e.g., "meter", "kilogram", "liter")
+     * @return the MeasureUnit if valid, otherwise null
+     * @internal
+     * @deprecated This API is ICU internal only.
+     */
+    @Deprecated
+    public static MeasureUnit getUnit(String type, String subtype) {
+        populateCache();
+        Map<String, MeasureUnit> units = cache.get(type);
+        return units != null ? units.get(subtype) : null;
+    }
+
     static final UnicodeSet ASCII = new UnicodeSet('a', 'z').freeze();
     static final UnicodeSet ASCII_HYPHEN_DIGITS =
             new UnicodeSet('-', '-', '0', '9', 'a', 'z').freeze();