Codereview request for 7096080: UTF8 update and new CESU-8 charset

Ulf Zibis Ulf.Zibis at gmx.de
Mon Nov 7 20:57:52 UTC 2011


Hi Sherman,

2 nits again:
protected static final class Encoder extends CharsetEncoder
- If final, protected doesn't make sense, as it can't be subclassed. (ideally this should be a 
compiler error)
- Where is it used? Otherwise it could be private.

-Ulf


Am 07.11.2011 21:30, schrieb Xueming Shen:
> Thanks Alan,
>
> The webrev has been updated accordingly to address your comments (the
> commented out isMalformed2 has been removed, copyright updated, bugid
> added...)
>
> And thanks for Ulf for the detailed review.
>
> -Sherman
>
> On 10/28/2011 09:14 AM, Alan Bateman wrote:
>> On 28/09/2011 20:18, Xueming Shen wrote:
>>> Hi,
>>>
>>> [I combined the proposed charge for #7082884, in which no one appears to be
>>> interested:-) into this one]
>>> :
>>>
>>> http://cr.openjdk.java.net/~sherman/7096080/webrev/ 
>>> <http://cr.openjdk.java.net/%7Esherman/7096080/webrev/>
>> I don't know if you are still looking for a reviewer for this (seems like Ulf has gone through 
>> this in detail, thanks Ulf).
>>
>> Overall it looks fine to me. Minor comment is that in UTF_8.java then maybe isMalformed2 should 
>> be removed completely, maybe move some of the comment in the decode methods. Another minor nits 
>> is that the date on CESU_8.java is 2000-2010 where I assume it should be 2011. In Errors.java 
>> then it might be better to just remove L196 as it might confuse future maintainers. I would also 
>> suggest adding the bugID to the list of bugs in the tests too as someone these references are 
>> useful.
>>
>> -Alan.
>
>



More information about the core-libs-dev mailing list