On 10/28/2014 03:52 PM, Alan Bateman wrote:
On 26/10/2014 21:10, Richard Warburton wrote:
:

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/ <http://cr.openjdk.java.net/%7Erwarburton/string-patch-webrev-8/>.

I think this is looking good.

For the constructor then the words "decoding the specified byte buffer", it might be a bit clearer as "decoding the remaining bytes in the ...".

For getBytes(ByteBuffer, Charset) then the position is advanced by the bytes written, no need to mention the number of chars read here.

In the constructor then you make it clear that malformed/unmappable sequences use the default replacement. This is important to state in the getBytes methods too because the encoding can fail.

-Alan.



Hi Richard, hi all,
Two comments,
You replace the nullcheck in getBytes() by a requireNonNull and at the same time, you don"t use requireNonNull in String(ByteBuffer,Charset),
no very logical isn't it ?
I think you need a supplementary constructor that takes a ByteBuffer and a charset name as a String, i may be wrong, but it seems that every constructor of String that takes a Charset has a dual constructor that takes a Charset as a String. As far as I remember, a constructor that takes a Charset as a String may be faster because you can share the character decoder
instead of creating a new one.

cheers,
RĂ©mi

Reply via email to