[PATCH] RuleBasedCollator
Guilhem Lavaux
guilhem@kaffe.org
Thu Dec 18 17:35: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.
>
>
Thank you. ;-)
>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.
>
>
Ok.
>
>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".
>
>
There isn't any reason. It's simply I should have forgotten to mark it
final again when I've merged source code.
>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.
>
>
I'll replace it with an "if" if it doesn't pose problems (it's sometimes
difficult to find how to use try ... catch).
Thank you very much for taking the time of reviewing it.
Guilhem.
More information about the Java-patches
mailing list