This is the mail archive of the
mailing list for the Java project.
Re: [PATCH] RuleBasedCollator
- From: Guilhem Lavaux <guilhem at kaffe dot org>
- To: tromey at redhat dot com
- Cc: java-patches at gcc dot gnu dot org, Classpath List <classpath at gnu dot org>
- Date: Thu, 18 Dec 2003 18:35:06 +0100
- Subject: Re: [PATCH] RuleBasedCollator
- References: <3FDB6112.firstname.lastname@example.org> <email@example.com>
Tom Tromey wrote:
Thank you. ;-)
"Guilhem" == Guilhem Lavaux <firstname.lastname@example.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.
There isn't any reason. It's simply I should have forgotten to mark it
final again when I've merged source code.
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
I'll replace it with an "if" if it doesn't pose problems (it's sometimes
difficult to find how to use try ... catch).
Guilhem> // Hehehe. What we would do not to use if.
Guilhem> ord1 = ord1block.getValue();
Guilhem> catch (NullPointerException _)
Guilhem> if (ord2block == null)
Guilhem> return 0;
Guilhem> return -1;
I'd prefer a simple "if" as I find it clearer to read.
Thank you very much for taking the time of reviewing it.