ICU-8295 changes based on code review

X-SVN-Rev: 30076

diff --git a/icu4j/main/classes/collate/src/com/ibm/icu/text/RuleBasedCollator.java b/icu4j/main/classes/collate/src/com/ibm/icu/text/RuleBasedCollator.java
index f41182699d7a0bcded84ab18021fdf75cf5310de..aca310eedcaff503782999b45b01da20f227685d 100644 (file)
--- a/icu4j/main/classes/collate/src/com/ibm/icu/text/RuleBasedCollator.java
+++ b/icu4j/main/classes/collate/src/com/ibm/icu/text/RuleBasedCollator.java
@@ -761,12 +761,29 @@ public final class RuleBasedCollator extends Collator {
 
     /** 
      * Sets the reordering codes for this collator.
-     * Reordering codes allow the collation ordering for groups of characters to be changed.
-     * The reordering codes are a combination of UScript  codes and ReorderCodes.
-     * These allow the ordering of characters belonging to these groups to be changed as a group.  
+     * Collation reordering allows scripts and some other defined blocks of characters 
+     * to be moved relative to each other as a block. This reordering is done on top of 
+     * the DUCET/CLDR standard collation order. Reordering can specify groups to be placed 
+     * at the start and/or the end of the collation order.
+     * <p>By default, reordering codes specified for the start of the order are placed in the 
+     * order given after a group of “special” non-script blocks. These special groups of characters 
+     * are space, punctuation, symbol, currency, and digit. These special groups are represented with
+     * {@link Collator.ReorderCodes}. Script groups can be intermingled with 
+     * these special non-script blocks if those special blocks are explicitly specified in the reordering.
+     * <p>The special code {@link Collator.ReorderCodes#OTHERS OTHERS} stands for any script that is not explicitly 
+     * mentioned in the list of reordering codes given. Anything that is after {@link Collator.ReorderCodes#OTHERS OTHERS}
+     * will go at the very end of the reordering in the order given.
+     * <p>The special reorder code {@link Collator.ReorderCodes#DEFAULT DEFAULT} will reset the reordering for this collator
+     * to the default for this collator. The default reordering may be the DUCET/CLDR order or may be a reordering that
+     * was specified when this collator was created from resource data or from rules. The 
+     * {@link Collator.ReorderCodes#DEFAULT DEFAULT} code <b>must</b> be the sole code supplied when it used. If not
+     * that will result in an {@link IllegalArgumentException} being thrown.
+     * <p>The special reorder code {@link Collator.ReorderCodes#NONE NONE} will remove any reordering for this collator.
+     * The result of setting no reordering will be to have the DUCET/CLDR reordering used. The 
+     * {@link Collator.ReorderCodes#NONE NONE} code <b>must</b> be the sole code supplied when it used.
      * @param order the reordering codes to apply to this collator; if this is null or an empty array
      * then this clears any existing reordering
-     * @throws IllegalArgumentException if the reordering codes are malformed in any (e.g. duplicates, multiple reset codes, overlapping equivalent scripts)
+     * @throws IllegalArgumentException if the reordering codes are malformed in any way (e.g. duplicates, multiple reset codes, overlapping equivalent scripts)
      * @see #getReorderCodes
      * @see #getEquivalentReorderCodes
      * @draft ICU 4.8
@@ -3980,6 +3997,9 @@ public final class RuleBasedCollator extends Collator {
         }
 
         if (m_reorderCodes_[0] == ReorderCodes.DEFAULT) {
+            if (m_reorderCodes_.length != 1) {
+                throw new IllegalArgumentException("Illegal collation reorder codes - default reorder code must be the only code in the list.");                
+            }
             // swap the reorder codes for those at build of the rules
             if (m_defaultReorderCodes_ == null || m_defaultReorderCodes_.length == 0) {
                 m_leadBytePermutationTable_ = null;                

diff --git a/icu4j/main/tests/collate/src/com/ibm/icu/dev/test/collator/CollationMiscTest.java b/icu4j/main/tests/collate/src/com/ibm/icu/dev/test/collator/CollationMiscTest.java
index ae61e19f3f0648df45b1175fa68605017e23c5cc..bd08413c37e9cc1d46f3ce41c0c49e0e143f65fa 100644 (file)
--- a/icu4j/main/tests/collate/src/com/ibm/icu/dev/test/collator/CollationMiscTest.java
+++ b/icu4j/main/tests/collate/src/com/ibm/icu/dev/test/collator/CollationMiscTest.java
@@ -3174,6 +3174,7 @@ public class CollationMiscTest extends TestFmwk {
         Collator myCollation;
         int[] reorderCodes = {UScript.GREEK, UScript.HAN, ReorderCodes.PUNCTUATION};
         int[] duplicateReorderCodes = {UScript.CUNEIFORM, UScript.GREEK, ReorderCodes.CURRENCY, UScript.EGYPTIAN_HIEROGLYPHS};
+        int[] reorderCodesStartingWithDefault = {ReorderCodes.DEFAULT, UScript.GREEK, UScript.HAN, ReorderCodes.PUNCTUATION};
         int[] retrievedReorderCodes;
         String greekString = "\u03b1";
         String punctuationString = "\u203e";
@@ -3239,6 +3240,17 @@ public class CollationMiscTest extends TestFmwk {
         if (!gotException) {
             errln("ERROR: exception was not thrown for illegal reorder codes argument.");            
         }
+        
+        /* set duplicate reorder codes */
+        gotException = false;
+        try {
+            myCollation.setReorderCodes(reorderCodesStartingWithDefault);
+        } catch (IllegalArgumentException e) {
+            gotException = true;
+        }
+        if (!gotException) {
+            errln("ERROR: reorder codes following a 'default' code should have thrown an exception but did not.");            
+        }
     }
     
     /*