On 19/10/2014 23:13, Richard Warburton wrote:
Hi,
Hi Richard, couple comments after a quick scan.
Thanks for your comments - only just had a post Javaone chance to look at
this stuff. I've pushed an update to:
*http://cr.openjdk.java.net/~rwarburton/string-patch-webrev-7
<http://cr.openjdk.java.net/~rwarburton/string-patch-webrev-7>*
I think this looks better but I have a few comments on the API.
For String(ByteBuffer, Charset) then it's still inconsistent to read
from the buffer position but not advance it. I think the constructor
needs to work like a regular decode in that respect. Related to this is
the underflow case where there are insufficient remaining bytes to
complete. If you don't advance the position then there is no way to
detect this.
The statement about the length of the String ".. may not be equal to the
length of the subarray" might be there from a previous iteration. There
isn't any array in the method signature so I think this statement needs
to be make a bit clearer.
For getBytes(byte[], int, int, Charset) then I think the javadoc could
say a bit more. It will encode to a maximum of destBuffer.length -
destOffset for example. The @return should probably say that it's the
number of bytes written to the array rather than "copied". Minor nits
is that you probably don't want the starting <p>. Also the finals in the
signature seem noisy/not needed.
The getBytes(ByteBuffer, Charset) method needs a bit more javadoc. You
should be able to borrow from text from the CharsetEncoder#encode
methods to help with that.
-Alan.
