RFR: 8372353: API to compute the byte length of a String encoded in a given Charset [v2]
Liam Miller-Cushon
cushon at openjdk.org
Thu Jan 15 09:13:18 UTC 2026
On Wed, 14 Jan 2026 22:49:06 GMT, Roger Riggs <rriggs at openjdk.org> wrote:
>> Liam Miller-Cushon has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Review feedback
>
> src/java.base/share/classes/java/lang/String.java line 1080:
>
>> 1078: return value.length;
>> 1079: }
>> 1080: int len = value.length >> 1;
>
> I don't think I understand what's being done and what Charset encoder it is mimicking.
> It probably needs to document the assumptions about unmappable characters and malformed surrogates.
> (Likely it is correct since the test of US_ASCII passes, but could use an explanation).
I added some `//` comments documenting which methods the `encodedLength*` methods are mimicking. The logic here should be identical to `encodeASCII` (except that it isn't allocating and writing to a destination array).
The handling of unmappable characters and malformed surrogates should match `encodeASCII`.
> src/java.base/share/classes/java/lang/String.java line 2130:
>
>> 2128:
>> 2129: /**
>> 2130: * Returns the length in bytes of the given String encoded with the given {@link Charset}.
>
> You can use the javadoc tag `@return` and skip the duplication. This first sentence reads better then the @return below since it emphasies the "encoded string" aspect.
Sorry I'm not sure I understand, can you clarify how that would work?
The javadoc can't start with `@return`, it needs to be a non-tag sentence fragment (the build enables doclint to enforce this).
> src/java.base/share/classes/java/lang/String.java line 2136:
>
>> 2134: * @implNote This method may allocate memory to compute the length for some charsets.
>> 2135: *
>> 2136: * @param cs the charset used to the compute the length
>
> Capitalize and perhaps link "Charset".
Done
> src/java.base/share/classes/java/lang/String.java line 2155:
>
>> 2153: }
>> 2154:
>> 2155: private int byteFor(int length, int coder) {
>
> Add a //comment please.
> The method name should be plural. `bytesForCoder`.
Done
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693532105
PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693479863
PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693480259
PR Review Comment: https://git.openjdk.org/jdk/pull/28454#discussion_r2693532859
More information about the core-libs-dev
mailing list