Re: [PATCH] RuleBasedCollator

[Tom Tromey]

>>>>> "Guilhem" == Guilhem Lavaux <address@hidden> 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.

Tom