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?
-Alan
