review request 6933322 - Add methods highSurrogate(), lowSurrogate() to class Character

Masayoshi Okutsu Masayoshi.Okutsu at Sun.COM
Thu Apr 1 08:42:26 UTC 2010


Hi Martin,

Here are my comments on the 6933322 changes.

- I'd suggest that the Unicode terms be used instead of "the first part" 
and "the second part", something like "the high surrogate (also known as 
leading surrogate) code unit of the surrogate pair." If you want to 
emphasize the order, "the leading surrogate (high surrogate) code unit" 
should be OK. Actually there were some discussions about high/low vs. 
leading/trailing in JSR 204, and we decided to use high/low to follow 
the (main) Unicode terms.

http://www.unicode.org/glossary/#high_surrogate_code_unit
http://www.unicode.org/glossary/#leading_surrogate

- @param, @return and @since are missing. There should be @see for the 
counterpart.

Otherwise, the changes look good to me, assuming that CCC would approve 
the API change.

Thanks,
Masayoshi

On 3/26/2010 5:52 AM, Ulf Zibis wrote:
> Updated topic.
>
> -Ulf
>
>
> Am 25.03.2010 21:42, schrieb Ulf Zibis:
>> Am 24.03.2010 09:24, schrieb Martin Buchholz:
>>> Ulf, Sherman, Masayoshi,
>>> here are changes for you to review.
>>> Only the patch highSurrogate needs a separate bug filed
>>> (and CCC, please)
>>
>> I had just filed it 2 weeks ago, see:
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6933322
>>
>> -Ulf



More information about the core-libs-dev mailing list