[PATCH] RuleBasedCollator

Guilhem Lavaux guilhem@kaffe.org
Sat Dec 20 16:44:00 GMT 2003


Tom Tromey wrote:

>>>>>>"Guilhem" == Guilhem Lavaux <guilhem@kaffe.org> writes:
>>>>>>            
>>>>>>
>
>Guilhem> Please review.
>
>I read through this.  The code looks fine to me, based on what I
>remember of RuleBasedCollator.  I'm glad to see that someone is paying
>attention to java.text -- this is some of our oldest and
>least-worked-on code, it really deserves a face-lift.
>
>
>There were a few formatting issues: lines past 79 columns that should
>be wrapped, and lack of whitespace around some operators, eg:
>
>Guilhem>     for (i=0;i<prefix.length() && i < s.length();i++)
>
>You need spaces around "=", "<", and after ";" here.
>
>
>I also had a couple other questions.
>
>Guilhem>   class CollationElement
>
>You changed this from a final class, but I didn't see anything derived
>from it.  Any reason why?  I find marking helpers like this "final"
>nice because it is a way to say "don't worry about subclasses if you
>change this".
>
>Guilhem> 	// Hehehe. What we would do not to use if.
>Guilhem> 	try
>Guilhem> 	  {
>Guilhem> 	    ord1 = ord1block.getValue();
>Guilhem> 	  }
>Guilhem> 	catch (NullPointerException _)
>Guilhem> 	  {
>Guilhem> 	    if (ord2block == null)
>Guilhem> 	      return 0;
>Guilhem> 	    return -1;
>Guilhem> 	  }
>
>I'd prefer a simple "if" as I find it clearer to read.
>  
>

Here are the two new files containing the suggested fixes. BTW, I've 
added some documentation for the internal methods.

Regards,
Guilhem.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: RuleBasedCollator.java
URL: <http://gcc.gnu.org/pipermail/java-patches/attachments/20031220/895be9b3/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: CollationElementIterator.java
URL: <http://gcc.gnu.org/pipermail/java-patches/attachments/20031220/895be9b3/attachment-0001.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 252 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/java-patches/attachments/20031220/895be9b3/attachment.sig>


More information about the Java-patches mailing list