RFR: [15] JDK-8236030: Cleanup use of String.toCharArray

[Claes Redestad]

Sorry for jumping in: I initially misread Hannes' observation in the
same way, but I think he meant that "   ".strip() will return "", while
removeTrailingWhitespace("    ") will return "    ", which is a more
subtle difference in behavior.

/Claes

On 2019-12-18 15:46, Jonathan Gibbons wrote:
> Hannes,
> 
> Thanks for the review.
> 
> My reading of the code for String.strip.... is that the original string 
> is returned if there is no whitespace to be removed ... in other words, 
> the new methods should have the same behavior as the removed methods.
> 
> -- Jon
> 
> On 12/18/19 4:56 AM, Hannes Wallnöfer wrote:
>> Looks good.
>>
>> I noticed that the removed methods would return the original string if 
>> they didn’t encounter a non-whitespace character, but I guess that 
>> wasn’t actually part of their intended behaviour.
>>
>> Hannes
>>
>>> Am 16.12.2019 um 23:47 schrieb Jonathan Gibbons 
>>> <jonathan.gibbons at oracle.com>:
>>>
>>> Updated webrev based on Ivan's suggestion to use one of 
>>> stripLeading(), stripTrailing(), or strip() instead of the old 
>>> methods, which are now removed
>>>
>>> -- Jon
>>>
>>> http://cr.openjdk.java.net/~jjg/8236030/webrev.01/
>>>
>>>
>>>
>>> On 12/16/2019 01:33 PM, Jonathan Gibbons wrote:
>>>> Ivan,
>>>>
>>>> Great suggestion, and sets up the possibility to  use strip() when 
>>>> both leading and trailing whitespace should be removed.
>>>>
>>>> The only slight change in semantics would be that these methods work 
>>>> in terms of code-points, not characters, and javadoc has not (yet?) 
>>>> been adapted to use code-points throughout.
>>>>
>>>> -- Jon
>>>>
>>>>
>>>> On 12/16/2019 01:23 PM, Ivan Gerasimov wrote:
>>>>> Hello!
>>>>>
>>>>> Can String.stripLeading()/stripTrailing() methods be used instead, 
>>>>> or is there a reason to avoid them?
>>>>>
>>>>> With kind regards,
>>>>>
>>>>> Ivan
>>>>>
>>>>>
>>>>> On 12/16/19 1:10 PM, Jonathan Gibbons wrote:
>>>>>> Please review a tiny change to eliminate a string copy into a 
>>>>>> temporary character buffer when removing leading or trailing 
>>>>>> whitespace from a string.
>>>>>>
>>>>>> The affected methods are called within the main code to translate 
>>>>>> doc comments to HTML.  This is noreg-cleanup/no-reg-trivial, so no 
>>>>>> new additional tests.
>>>>>>
>>>>>> Webrev below, but the patch is also included here:
>>>>>>
>>>>>> --- 
>>>>>> a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java 
>>>>>> Mon Dec 16 15:33:03 2019 -0500
>>>>>> +++ 
>>>>>> b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java 
>>>>>> Mon Dec 16 13:07:18 2019 -0800
>>>>>> @@ -1604,22 +1604,19 @@
>>>>>>       }
>>>>>>
>>>>>>       private String removeTrailingWhitespace(String text) {
>>>>>> -        char[] buf = text.toCharArray();
>>>>>> -        for (int i = buf.length - 1; i > 0 ; i--) {
>>>>>> -            if (!Character.isWhitespace(buf[i]))
>>>>>> -                return text.substring(0, i + 1);
>>>>>> +        int i = text.length() - 1;
>>>>>> +        while (i >= 0 && Character.isWhitespace(text.charAt(i))) {
>>>>>> +            i--;
>>>>>>           }
>>>>>> -        return text;
>>>>>> +        return i == text.length() - 1 ? text : text.substring(0, 
>>>>>> i + 1);
>>>>>>       }
>>>>>>
>>>>>>       private String removeLeadingWhitespace(String text) {
>>>>>> -        char[] buf = text.toCharArray();
>>>>>> -        for (int i = 0; i < buf.length; i++) {
>>>>>> -            if (!Character.isWhitespace(buf[i])) {
>>>>>> -                return text.substring(i);
>>>>>> -            }
>>>>>> +        int i = 0;
>>>>>> +        while (i < text.length() && 
>>>>>> Character.isWhitespace(text.charAt(i))) {
>>>>>> +            i++;
>>>>>>           }
>>>>>> -        return text;
>>>>>> +        return i == 0 ? text : text.substring(i);
>>>>>>       }
>>>>>>
>>>>>>       /**
>>>>>>
>>>>>>
>>>>>> -- Jon
>>>>>>
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8236030
>>>>>> Webrev: http://cr.openjdk.java.net/~jjg/8236030/webrev/
>>>>>>