From 449ad53d7c202b5606ac8d5138cd8d7ffde1421a Mon Sep 17 00:00:00 2001 From: Doug Felt Date: Fri, 18 Feb 2011 01:42:10 +0000 Subject: [PATCH] ICU-8158 change implementation, add some tests, make serialization compatible with previous X-SVN-Rev: 29457 --- .../src/com/ibm/icu/text/PluralRules.java | 169 +++++++++++------- .../icu/dev/test/format/PluralRulesTest.java | 58 ++++-- 2 files changed, 147 insertions(+), 80 deletions(-) diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/PluralRules.java b/icu4j/main/classes/core/src/com/ibm/icu/text/PluralRules.java index 7e11fb7823..9d41aa1f74 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/PluralRules.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/PluralRules.java @@ -12,6 +12,7 @@ import java.text.ParseException; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Locale; import java.util.Map; @@ -79,8 +80,10 @@ public class PluralRules implements Serializable { private static final long serialVersionUID = 1; private final RuleList rules; - private final Map keywords; + private final Set keywords; private int repeatLimit; // for equality test + private transient Map uniqueKeywordValues; + private transient int hashCode; // Standard keywords. @@ -122,6 +125,13 @@ public class PluralRules implements Serializable { */ public static final String KEYWORD_OTHER = "other"; + /** + * Value returned by {@link #getUniqueKeywordValue} when there is no + * unique value to return. + * @defat ICU 4.8 + */ + public static final double NO_UNIQUE_VALUE = -0.00123456777; + /* * The set of all characters a valid keyword can start with. */ @@ -144,6 +154,7 @@ public class PluralRules implements Serializable { public boolean isFulfilled(double n) { return true; } + public String toString() { return "n is any"; } @@ -151,9 +162,6 @@ public class PluralRules implements Serializable { public int updateRepeatLimit(int limit) { return limit; } - public double uniqueValue() { - return Double.NaN; - } }; /* @@ -178,8 +186,9 @@ public class PluralRules implements Serializable { return limit; } - public Double getKeywordValue() { - return Double.NaN; + public double getKeywordValue() { + // TODO Auto-generated method stub + return 0; } }; @@ -235,11 +244,6 @@ public class PluralRules implements Serializable { */ boolean isFulfilled(double n); - /* - * Returns the unique value that satisfies the constraint, or NaN. - */ - double uniqueValue(); - /* * Returns the larger of limit or the limit of this constraint. * If the constraint is a simple range test, this is the higher @@ -257,12 +261,12 @@ public class PluralRules implements Serializable { private interface Rule extends Serializable { /* Returns the keyword that names this rule. */ String getKeyword(); + /* Returns true if the rule applies to the number. */ boolean appliesTo(double n); + /* Returns the larger of limit and this rule's limit. */ int updateRepeatLimit(int limit); - /* Returns the unique value that satisfies this rule, or NaN if there is no such value */ - Double getKeywordValue(); } /* @@ -273,7 +277,7 @@ public class PluralRules implements Serializable { String select(double n); /* Returns the set of defined keywords. */ - Map getKeywords(); + Set getKeywords(); /* Return the value at which this rulelist starts repeating. */ int getRepeatLimit(); @@ -502,13 +506,45 @@ public class PluralRules implements Serializable { } public String toString() { - return "[mod: " + mod + " inRange: " + inRange + - " integersOnly: " + integersOnly + - " low: " + lowerBound + " high: " + upperBound + "]"; - } - - public double uniqueValue() { - return mod == 0 && lowerBound == upperBound ? lowerBound : Double.NaN; + class ListBuilder { + StringBuilder sb = new StringBuilder("["); + ListBuilder add(String s) { + return add(s, null); + } + ListBuilder add(String s, Object o) { + if (sb.length() > 1) { + sb.append(", "); + } + sb.append(s); + if (o != null) { + sb.append(": ").append(o.toString()); + } + return this; + } + public String toString() { + String s = sb.append(']').toString(); + sb = null; + return s; + } + } + ListBuilder lb = new ListBuilder(); + if (mod > 1) { + lb.add("mod", mod); + } + if (inRange) { + lb.add("in"); + } else { + lb.add("except"); + } + if (integersOnly) { + lb.add("ints"); + } + if (lowerBound == upperBound) { + lb.add(String.valueOf(lowerBound)); + } else { + lb.add(String.valueOf(lowerBound) + "-" + String.valueOf(upperBound)); + } + return lb.toString(); } } @@ -543,16 +579,6 @@ public class PluralRules implements Serializable { super(a, b, " && "); } - public double uniqueValue() { - if (Double.isNaN(a.uniqueValue())) { - return b.uniqueValue(); - } - if (Double.isNaN(b.uniqueValue())) { - return a.uniqueValue(); - } - return a.uniqueValue() == b.uniqueValue() ? a.uniqueValue() : Double.NaN; // degenerate case - } - public boolean isFulfilled(double n) { return a.isFulfilled(n) && b.isFulfilled(n); } @@ -566,10 +592,6 @@ public class PluralRules implements Serializable { super(a, b, " || "); } - public double uniqueValue() { - return a.uniqueValue() == b.uniqueValue() ? (double)a.uniqueValue() : Double.NaN; - } - public boolean isFulfilled(double n) { return a.isFulfilled(n) || b.isFulfilled(n); } @@ -582,12 +604,10 @@ public class PluralRules implements Serializable { private static class ConstrainedRule implements Rule, Serializable { private static final long serialVersionUID = 1; private final String keyword; - private final double keywordValue; private final Constraint constraint; public ConstrainedRule(String keyword, Constraint constraint) { this.keyword = keyword; - this.keywordValue = constraint.uniqueValue(); this.constraint = constraint; } @@ -616,10 +636,6 @@ public class PluralRules implements Serializable { public String toString() { return keyword + ": " + constraint; } - - public Double getKeywordValue() { - return keywordValue; - } } /* @@ -664,17 +680,16 @@ public class PluralRules implements Serializable { return r.getKeyword(); } - public Map getKeywords() { - Map result = new HashMap(); - result.put(KEYWORD_OTHER, Double.NaN); + public Set getKeywords() { + Set result = new HashSet(); + result.add(KEYWORD_OTHER); RuleChain rc = this; while (rc != null) { - result.put(rc.rule.getKeyword(), rc.rule.getKeywordValue()); + result.add(rc.rule.getKeyword()); rc = rc.next; } return result; } - public int getRepeatLimit() { int result = 0; RuleChain rc = this; @@ -741,7 +756,7 @@ public class PluralRules implements Serializable { */ private PluralRules(RuleList rules) { this.rules = rules; - this.keywords = Collections.unmodifiableMap(rules.getKeywords()); + this.keywords = Collections.unmodifiableSet(rules.getKeywords()); } /** @@ -764,19 +779,37 @@ public class PluralRules implements Serializable { * @stable ICU 3.8 */ public Set getKeywords() { - return keywords.keySet(); + return keywords; } /** - * Returns the unique value that this keyword matches, or NaN if the keyword matches - * multiple values or is not defined for this PluralRules. + * Returns the unique value that this keyword matches, or {@link #NO_UNIQUE_VALUE} + * if the keyword matches multiple values or is not defined for this PluralRules. * * @param keyword the keyword to check for a unique value - * @return The unique value for the keyword, or NaN. + * @return The unique value for the keyword, or NO_UNIQUE_VALUE. * @draft ICU 4.8 */ public double getUniqueKeywordValue(String keyword) { - return keywords.containsKey(keyword) ? keywords.get(keyword) : Double.NaN; + if (uniqueKeywordValues == null) { + // compute unique values the slow but exact way, the logic to do this from the rules is more complex than + // this simple process + final Double NONE = NO_UNIQUE_VALUE; + uniqueKeywordValues = new HashMap(); + int limit = getRepeatLimit(); + for (int i = 0; i < limit * 2; ++i) { + for (int j = 0; j < 2; ++j) { + double value = i + 0.5 * j; + String key = select(value); + if (uniqueKeywordValues.containsKey(key)) { + uniqueKeywordValues.put(key, NONE); + } else { + uniqueKeywordValues.put(key, value); + } + } + } + } + return uniqueKeywordValues.containsKey(keyword) ? uniqueKeywordValues.get(keyword) : NO_UNIQUE_VALUE; } /** @@ -787,7 +820,7 @@ public class PluralRules implements Serializable { * @deprecated This API is ICU internal only. */ public Collection getSamples(String keyword, int max) { - if (!keywords.containsKey(keyword)) { + if (!keywords.contains(keyword)) { return null; } LinkedHashSet results = new LinkedHashSet(); @@ -851,8 +884,9 @@ public class PluralRules implements Serializable { * @stable ICU 3.8 */ public String toString() { - return "keywords: " + keywords + " rules: " + rules.toString() + - " limit: " + getRepeatLimit(); + return "keywords: " + keywords + + " limit: " + getRepeatLimit() + + " rules: " + rules.toString(); } /** @@ -860,7 +894,18 @@ public class PluralRules implements Serializable { * @stable ICU 3.8 */ public int hashCode() { - return keywords.hashCode(); + if (hashCode == 0) { + // cache it + int newHashCode = keywords.hashCode(); + for (int i = 0; i < 12; ++i) { + newHashCode = newHashCode * 31 + select(i).hashCode(); + } + if (newHashCode == 0) { + newHashCode = 1; + } + hashCode = newHashCode; + } + return hashCode; } /** @@ -885,18 +930,16 @@ public class PluralRules implements Serializable { return true; } + if (hashCode() != rhs.hashCode()) { + return false; + } + if (!getKeywords().equals(rhs.getKeywords())) { // note, we're comparing keys only here return false; } - // can't compare maps for equality since values might include NaN and Nan != itself. - for (String k : getKeywords()) { - double val = getUniqueKeywordValue(k); - double rhsVal = rhs.getUniqueKeywordValue(k); - return Double.isNaN(val) ? Double.isNaN(rhsVal) : val == rhsVal; - } int limit = Math.max(getRepeatLimit(), rhs.getRepeatLimit()); - for (int i = 0; i < limit; ++i) { + for (int i = 0; i < limit * 2; ++i) { if (!select(i).equals(rhs.select(i))) { return false; } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java index bcd20c0305..c4f838d64f 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/PluralRulesTest.java @@ -106,34 +106,58 @@ public class PluralRulesTest extends TestFmwk { } private static String[][] equalityTestData = { + { "a: n is 5", + "a: n in 2..6 and n not in 2..4 and n is not 6" }, { "a: n in 2..3", - "a: n is 2 or n is 3", - "a:n is 3 and n in 2..5 or n is 2" }, + "a: n is 2 or n is 3", + "a: n is 3 and n in 2..5 or n is 2" }, { "a: n is 12; b:n mod 10 in 2..3", "b: n mod 10 in 2..3 and n is not 12; a: n in 12..12", - "b: n is 13; a: n in 12..13; b: n mod 10 is 2 or n mod 10 is 3" }, + "b: n is 13; a: n is 12; b: n mod 10 is 2 or n mod 10 is 3" }, + }; + + private static String[][] inequalityTestData = { + { "a: n mod 8 is 3", + "a: n mod 7 is 3" + }, + { "a: n mod 3 is 2 and n is not 5", + "a: n mod 6 is 2 or n is 8 or n is 11" + } }; - private void compareEquality(Object[] objects) { + private void compareEquality(String id, Object[] objects, boolean shouldBeEqual) { for (int i = 0; i < objects.length; ++i) { Object lhs = objects[i]; - for (int j = i; j < objects.length; ++j) { + int start = shouldBeEqual ? i : i + 1; + for (int j = start; j < objects.length; ++j) { Object rhs = objects[j]; - assertEquals("obj " + i + " and " + j, lhs, rhs); + if (shouldBeEqual != lhs.equals(rhs)) { + String msg = shouldBeEqual ? "should be equal" : "should not be equal"; + fail(id + " " + msg + " (" + i + ", " + j + "):\n " + lhs + "\n " + rhs); + } + // assertEquals("obj " + i + " and " + j, lhs, rhs); } } } - public void testEquality() { - for (int i = 0; i < equalityTestData.length; ++i) { - String[] patterns = equalityTestData[i]; + private void compareEqualityTestSets(String[][] sets, boolean shouldBeEqual) { + for (int i = 0; i < sets.length; ++i) { + String[] patterns = sets[i]; PluralRules[] rules = new PluralRules[patterns.length]; for (int j = 0; j < patterns.length; ++j) { rules[j] = PluralRules.createRules(patterns[j]); } - compareEquality(rules); + compareEquality("test " + i, rules, shouldBeEqual); } } + + public void testEquality() { + compareEqualityTestSets(equalityTestData, true); + } + + public void testInequality() { + compareEqualityTestSets(inequalityTestData, false); + } public void testBuiltInRules() { // spot check @@ -247,15 +271,15 @@ public class PluralRulesTest extends TestFmwk { assertRuleValue("n is 1", 1); assertRuleValue("n in 2..2", 2); assertRuleValue("n within 2..2", 2); - assertRuleValue("n in 3..4", Double.NaN); - assertRuleValue("n within 3..4", Double.NaN); + assertRuleValue("n in 3..4", PluralRules.NO_UNIQUE_VALUE); + assertRuleValue("n within 3..4", PluralRules.NO_UNIQUE_VALUE); assertRuleValue("n is 2 or n is 2", 2); assertRuleValue("n is 2 and n is 2", 2); - assertRuleValue("n is 2 or n is 3", Double.NaN); - assertRuleValue("n is 2 and n is 3", Double.NaN); - assertRuleValue("n is 2 or n in 2..3", Double.NaN); + assertRuleValue("n is 2 or n is 3", PluralRules.NO_UNIQUE_VALUE); + assertRuleValue("n is 2 and n is 3", PluralRules.NO_UNIQUE_VALUE); + assertRuleValue("n is 2 or n in 2..3", PluralRules.NO_UNIQUE_VALUE); assertRuleValue("n is 2 and n in 2..3", 2); - assertRuleKeyValue("a: n is 1", "not_defined", Double.NaN); // key not defined - assertRuleKeyValue("a: n is 1", "other", Double.NaN); // key matches default rule + assertRuleKeyValue("a: n is 1", "not_defined", PluralRules.NO_UNIQUE_VALUE); // key not defined + assertRuleKeyValue("a: n is 1", "other", PluralRules.NO_UNIQUE_VALUE); // key matches default rule } }