From 6a1bbcaa5820bf097abf8bb4751da359bcbb1164 Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Sat, 16 Sep 2017 06:57:08 +0000 Subject: [PATCH] ICU-13177 Small Java changes relating to Compact and Padding. X-SVN-Rev: 40423 --- .../ibm/icu/text/CompactDecimalFormat.java | 20 -- .../core/src/newapi/CompactNotation.java | 68 ++++--- .../src/newapi/LocalizedNumberFormatter.java | 1 + .../core/src/newapi/NumberFormatter.java | 6 + .../core/src/newapi/NumberFormatterImpl.java | 7 +- .../core/src/newapi/impl/CompactData.java | 185 ++++++++---------- .../classes/core/src/newapi/impl/Padder.java | 3 +- .../dev/test/number/NumberFormatterTest.java | 11 ++ 8 files changed, 144 insertions(+), 157 deletions(-) diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/CompactDecimalFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/CompactDecimalFormat.java index 0a643cd7d96..7feeb33383c 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/CompactDecimalFormat.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/CompactDecimalFormat.java @@ -69,26 +69,6 @@ public class CompactDecimalFormat extends DecimalFormat { LONG } - /** - * Type parameter for CompactDecimalFormat. - * - * @draft ICU 60 - */ - public enum CompactType { - /** - * Standard compact format, like "1.2T" - * - * @draft ICU 60 - */ - DECIMAL, - /** - * Compact format with currency, like "$1.2T" - * - * @draft ICU 60 - */ - CURRENCY - } - /** * Creates a CompactDecimalFormat appropriate for a locale. The result may be affected by the * number system in the locale, such as ar-u-nu-latn. diff --git a/icu4j/main/classes/core/src/newapi/CompactNotation.java b/icu4j/main/classes/core/src/newapi/CompactNotation.java index 3df09789b7d..6f2415aae71 100644 --- a/icu4j/main/classes/core/src/newapi/CompactNotation.java +++ b/icu4j/main/classes/core/src/newapi/CompactNotation.java @@ -3,6 +3,7 @@ package newapi; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -11,7 +12,6 @@ import com.ibm.icu.impl.number.DecimalQuantity; import com.ibm.icu.impl.number.PatternStringParser; import com.ibm.icu.impl.number.PatternStringParser.ParsedPatternInfo; import com.ibm.icu.text.CompactDecimalFormat.CompactStyle; -import com.ibm.icu.text.CompactDecimalFormat.CompactType; import com.ibm.icu.text.PluralRules; import com.ibm.icu.util.ULocale; @@ -26,6 +26,10 @@ public class CompactNotation extends Notation { final CompactStyle compactStyle; final Map> compactCustomData; + public enum CompactType { + DECIMAL, CURRENCY + } + public CompactNotation(CompactStyle compactStyle) { compactCustomData = null; this.compactStyle = compactStyle; @@ -36,18 +40,13 @@ public class CompactNotation extends Notation { this.compactCustomData = compactCustomData; } - /* package-private */ MicroPropsGenerator withLocaleData(ULocale dataLocale, CompactType compactType, PluralRules rules, - MutablePatternModifier buildReference, MicroPropsGenerator parent) { - CompactData data; - if (compactStyle != null) { - data = CompactData.getInstance(dataLocale, compactType, compactStyle); - } else { - data = CompactData.getInstance(compactCustomData); - } - return new CompactImpl(data, rules, buildReference, parent); + /* package-private */ MicroPropsGenerator withLocaleData(ULocale locale, String nsName, CompactType compactType, + PluralRules rules, MutablePatternModifier buildReference, MicroPropsGenerator parent) { + // TODO: Add a data cache? It would be keyed by locale, nsName, compact type, and compact style. + return new CompactHandler(this, locale, nsName, compactType, rules, buildReference, parent); } - private static class CompactImpl implements MicroPropsGenerator { + private static class CompactHandler implements MicroPropsGenerator { private static class CompactModInfo { public ImmutablePatternModifier mod; @@ -55,28 +54,35 @@ public class CompactNotation extends Notation { } final PluralRules rules; - final CompactData data; - final Map precomputedMods; final MicroPropsGenerator parent; + final Map precomputedMods; + final CompactData data; - private CompactImpl(CompactData data, PluralRules rules, MutablePatternModifier buildReference, MicroPropsGenerator parent) { - this.data = data; + private CompactHandler(CompactNotation notation, ULocale locale, String nsName, CompactType compactType, + PluralRules rules, MutablePatternModifier buildReference, MicroPropsGenerator parent) { this.rules = rules; + this.parent = parent; + this.data = new CompactData(); + if (notation.compactStyle != null) { + data.populate(locale, nsName, notation.compactStyle, compactType); + } else { + data.populate(notation.compactCustomData); + } if (buildReference != null) { // Safe code path - precomputedMods = precomputeAllModifiers(data, buildReference); + precomputedMods = new HashMap(); + precomputeAllModifiers(buildReference); } else { // Unsafe code path precomputedMods = null; } - this.parent = parent; } /** Used by the safe code path */ - private static Map precomputeAllModifiers(CompactData data, - MutablePatternModifier buildReference) { - Map precomputedMods = new HashMap(); - Set allPatterns = data.getAllPatterns(); + private void precomputeAllModifiers(MutablePatternModifier buildReference) { + Set allPatterns = new HashSet(); + data.getUniquePatterns(allPatterns); + for (String patternString : allPatterns) { CompactModInfo info = new CompactModInfo(); ParsedPatternInfo patternInfo = PatternStringParser.parseToPatternInfo(patternString); @@ -85,27 +91,26 @@ public class CompactNotation extends Notation { info.numDigits = patternInfo.positive.integerTotal; precomputedMods.put(patternString, info); } - return precomputedMods; } @Override - public MicroProps processQuantity(DecimalQuantity input) { - MicroProps micros = parent.processQuantity(input); + public MicroProps processQuantity(DecimalQuantity quantity) { + MicroProps micros = parent.processQuantity(quantity); assert micros.rounding != null; // Treat zero as if it had magnitude 0 int magnitude; - if (input.isZero()) { + if (quantity.isZero()) { magnitude = 0; - micros.rounding.apply(input); + micros.rounding.apply(quantity); } else { // TODO: Revisit chooseMultiplierAndApply - int multiplier = micros.rounding.chooseMultiplierAndApply(input, data); - magnitude = input.isZero() ? 0 : input.getMagnitude(); + int multiplier = micros.rounding.chooseMultiplierAndApply(quantity, data); + magnitude = quantity.isZero() ? 0 : quantity.getMagnitude(); magnitude -= multiplier; } - StandardPlural plural = input.getStandardPlural(rules); + StandardPlural plural = quantity.getStandardPlural(rules); String patternString = data.getPattern(magnitude, plural); int numDigits = -1; if (patternString == null) { @@ -113,12 +118,13 @@ public class CompactNotation extends Notation { // No need to take any action. } else if (precomputedMods != null) { // Safe code path. + // Java uses a hash set here for O(1) lookup. C++ uses a linear search. CompactModInfo info = precomputedMods.get(patternString); - info.mod.applyToMicros(micros, input); + info.mod.applyToMicros(micros, quantity); numDigits = info.numDigits; } else { // Unsafe code path. - // Overwrite the PatternInfo in the existing modMiddle + // Overwrite the PatternInfo in the existing modMiddle. assert micros.modMiddle instanceof MutablePatternModifier; ParsedPatternInfo patternInfo = PatternStringParser.parseToPatternInfo(patternString); ((MutablePatternModifier) micros.modMiddle).setPatternInfo(patternInfo); diff --git a/icu4j/main/classes/core/src/newapi/LocalizedNumberFormatter.java b/icu4j/main/classes/core/src/newapi/LocalizedNumberFormatter.java index cd0956f13ba..b71f1a815bb 100644 --- a/icu4j/main/classes/core/src/newapi/LocalizedNumberFormatter.java +++ b/icu4j/main/classes/core/src/newapi/LocalizedNumberFormatter.java @@ -71,6 +71,7 @@ public class LocalizedNumberFormatter extends NumberFormatterSettings> powersToPluralsToPatterns) { - CompactData data = new CompactData(); + public void populate(Map> powersToPluralsToPatterns) { + assert isEmpty; for (Map.Entry> magnitudeEntry : powersToPluralsToPatterns.entrySet()) { byte magnitude = (byte) (magnitudeEntry.getKey().length() - 1); for (Map.Entry pluralEntry : magnitudeEntry.getValue().entrySet()) { StandardPlural plural = StandardPlural.fromString(pluralEntry.getKey().toString()); String patternString = pluralEntry.getValue().toString(); - data.setPattern(patternString, magnitude, plural); + patterns[getIndex(magnitude, plural)] = patternString; int numZeros = countZeros(patternString); if (numZeros > 0) { // numZeros==0 in certain cases, like Somali "Kun" - data.setMultiplier(magnitude, (byte) (numZeros - magnitude - 1)); + // Save the multiplier. + multipliers[magnitude] = (byte) (numZeros - magnitude - 1); + if (magnitude > largestMagnitude) { + largestMagnitude = magnitude; + } + isEmpty = false; } } } - return data; - } - - // A dummy object used when a "0" compact decimal entry is encountered. This is necessary - // in order to prevent falling back to root. Object equality ("==") is intended. - private static final String USE_FALLBACK = ""; - - private final String[] patterns; - private final byte[] multipliers; - private boolean isEmpty; - private int largestMagnitude; - - private static final int MAX_DIGITS = 15; - - private CompactData() { - patterns = new String[(CompactData.MAX_DIGITS + 1) * StandardPlural.COUNT]; - multipliers = new byte[CompactData.MAX_DIGITS + 1]; - isEmpty = true; - largestMagnitude = 0; - } - - public boolean isEmpty() { - return isEmpty; } @Override @@ -108,23 +110,6 @@ public class CompactData implements MultiplierProducer { return multipliers[magnitude]; } - /** Returns the multiplier from the array directly without bounds checking. */ - public int getMultiplierDirect(int magnitude) { - return multipliers[magnitude]; - } - - private void setMultiplier(int magnitude, byte multiplier) { - if (multipliers[magnitude] != 0) { - assert multipliers[magnitude] == multiplier; - return; - } - multipliers[magnitude] = multiplier; - isEmpty = false; - if (magnitude > largestMagnitude) { - largestMagnitude = magnitude; - } - } - public String getPattern(int magnitude, StandardPlural plural) { if (magnitude < 0) { return null; @@ -144,32 +129,13 @@ public class CompactData implements MultiplierProducer { return patternString; } - public Set getAllPatterns() { - Set result = new HashSet(); - result.addAll(Arrays.asList(patterns)); - result.remove(USE_FALLBACK); - result.remove(null); - return result; - } - - private boolean has(int magnitude, StandardPlural plural) { - // Return true if USE_FALLBACK is present - return patterns[getIndex(magnitude, plural)] != null; - } - - private void setPattern(String patternString, int magnitude, StandardPlural plural) { - patterns[getIndex(magnitude, plural)] = patternString; - isEmpty = false; - if (magnitude > largestMagnitude) - largestMagnitude = magnitude; - } - - private void setNoFallback(int magnitude, StandardPlural plural) { - setPattern(USE_FALLBACK, magnitude, plural); - } - - private static final int getIndex(int magnitude, StandardPlural plural) { - return magnitude * StandardPlural.COUNT + plural.ordinal(); + public void getUniquePatterns(Set output) { + assert output.isEmpty(); + // NOTE: In C++, this is done more manually with a UVector. + // In Java, we can take advantage of JDK HashSet. + output.addAll(Arrays.asList(patterns)); + output.remove(USE_FALLBACK); + output.remove(null); } private static final class CompactDataSink extends UResource.Sink { @@ -189,16 +155,17 @@ public class CompactData implements MultiplierProducer { // Assumes that the keys are always of the form "10000" where the magnitude is the // length of the key minus one. We expect magnitudes to be less than MAX_DIGITS. byte magnitude = (byte) (key.length() - 1); - byte multiplier = (byte) data.getMultiplierDirect(magnitude); - assert magnitude < MAX_DIGITS; + byte multiplier = data.multipliers[magnitude]; + assert magnitude < COMPACT_MAX_DIGITS; // Iterate over the plural variants ("one", "other", etc) UResource.Table pluralVariantsTable = value.getTable(); for (int i4 = 0; pluralVariantsTable.getKeyAndValue(i4, key, value); ++i4) { // Skip this magnitude/plural if we already have it from a child locale. + // Note: This also skips USE_FALLBACK entries. StandardPlural plural = StandardPlural.fromString(key.toString()); - if (data.has(magnitude, plural)) { + if (data.patterns[getIndex(magnitude, plural)] != null) { continue; } @@ -206,12 +173,11 @@ public class CompactData implements MultiplierProducer { // to parent locales. Example locale where this is relevant: 'it'. String patternString = value.toString(); if (patternString.equals("0")) { - data.setNoFallback(magnitude, plural); - continue; + patternString = USE_FALLBACK; } // Save the pattern string. We will parse it lazily. - data.setPattern(patternString, magnitude, plural); + data.patterns[getIndex(magnitude, plural)] = patternString; // If necessary, compute the multiplier: the difference between the magnitude // and the number of zeros in the pattern. @@ -223,11 +189,24 @@ public class CompactData implements MultiplierProducer { } } - data.setMultiplier(magnitude, multiplier); + // Save the multiplier. + if (data.multipliers[magnitude] == 0) { + data.multipliers[magnitude] = multiplier; + if (magnitude > data.largestMagnitude) { + data.largestMagnitude = magnitude; + } + data.isEmpty = false; + } else { + assert data.multipliers[magnitude] == multiplier; + } } } } + private static final int getIndex(int magnitude, StandardPlural plural) { + return magnitude * StandardPlural.COUNT + plural.ordinal(); + } + private static final int countZeros(String patternString) { // NOTE: This strategy for computing the number of zeros is a hack for efficiency. // It could break if there are any 0s that aren't part of the main pattern. diff --git a/icu4j/main/classes/core/src/newapi/impl/Padder.java b/icu4j/main/classes/core/src/newapi/impl/Padder.java index 9f9a0a6cd92..6180fd58ddb 100644 --- a/icu4j/main/classes/core/src/newapi/impl/Padder.java +++ b/icu4j/main/classes/core/src/newapi/impl/Padder.java @@ -117,7 +117,7 @@ public class Padder { // Should not happen since currency spacing is always on the inside. throw new AssertionError(); } - length += string.insert(insertIndex, paddingString, null); + addPaddingHelper(paddingString, requiredPadding, string, insertIndex); } return length; @@ -126,6 +126,7 @@ public class Padder { private static int addPaddingHelper(String paddingString, int requiredPadding, NumberStringBuilder string, int index) { for (int i = 0; i < requiredPadding; i++) { + // TODO: If appending to the end, this will cause actual insertion operations. Improve. string.insert(index, paddingString, null); } return paddingString.length() * requiredPadding; diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterTest.java index 44b651fb325..8a6d196be4c 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterTest.java @@ -1015,6 +1015,15 @@ public class NumberFormatterTest { ULocale.ENGLISH, 0.8888, "88.88%**"); + + assertFormatSingle( + "Currency Spacing with Zero Digit Padding Broken", + "$GBP unit-width=ISO_CODE", + NumberFormatter.with().padding(Padder.codePoints('0', 12, PadPosition.AFTER_PREFIX)).unit(GBP) + .unitWidth(UnitWidth.ISO_CODE), + ULocale.ENGLISH, + 514.23, + "GBP 000514.23"); // TODO: This is broken; it renders too wide (13 instead of 12). } @Test @@ -1330,6 +1339,8 @@ public class NumberFormatterTest { public void locale() { // Coverage for the locale setters. assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.with().locale(Locale.ENGLISH)); + assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.withLocale(ULocale.ENGLISH)); + assertEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.withLocale(Locale.ENGLISH)); assertNotEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.with().locale(Locale.FRENCH)); } -- 2.40.0