[PATCH] RuleBasedCollator
Tom Tromey
tromey@redhat.com
Thu Dec 18 17:16:00 GMT 2003
>>>>> "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.
Tom
More information about the Java-patches
mailing list