Hi Alan, I think this looks better but I have a few comments on the API. >
Thanks for taking the time to look at this - most appreciated. I've pushed the latest iteration to http://cr.openjdk.java.net/~rwarburton/string-patch-webrev-8/. 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. > I've modified the Javadoc and tests. The implementation was actually behaving this way already. 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. > I've rephrased this to refer to the ByteBuffer. 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. > I've updated the Javadoc with more information about the encoding and made these changes. I'm not sure if there's anything else that's missing in this case. regards, Richard Warburton http://insightfullogic.com @RichardWarburto <http://twitter.com/richardwarburto>
