On 11/24/2014 01:21 PM, Alan Bateman wrote:
On 24/11/2014 18:41, Xueming Shen wrote:
Hi Richard,
Here are some comments regarding the updated webrev.
(1) String(ByteBuffer, String) needs "@throw UEE".
(2) It should be "default replacement byte array" not "replace string" for all
getBytes() methods when malformed/unmappable
(3) for decoding (new String) from ByteBuffer, since it is guaranteed that all
bytes in the input ByteBuffer will be decoded, it might be desirable to
clearly
specify that the position of the buffer will be "advanced to its limit" ?
(4) ln#1137 has an extra "*"
(5) StringCoding.decode(cs, bb), why do you want to allocate a direct buffer
here for "untrusted"? Basically the output buffer "ca" will always be a
wrapper
of a char[], all our charset implementation will have better performance if
both input and output are "array based" (decode on array directly, instead
of
the "slow" ByteBuffer)
Overall I think this is looking quite good and I think Sherman has captured the main
issues. On #3 then wording such as ".. the position will be updated" isn't
clear enough to allow the method be tested, it needs to make it clear that the position
is advanced by the number of bytes that were decoded.
Sherman - are you going to sponsor this for Richard?
Yes, I will sponsor this RFE.
-Sherman
