RFR - JDK-8200434 - String::align, String::indent (code review)
roger.riggs at oracle.com
Wed Sep 5 19:50:56 UTC 2018
Overall it looks fine. Some quibbles on the wording and the test.
The spec for the align() and align(n) methods in String.java show a
possibly misleading inconsistency.
The first line says:
Removes vertical and horizontal white space margins.
But later align() says:
Trailing spaces are preserved.
The former implies all 4 margins but then it seems only to apply to 3 of
the 4. (top, left, bottom).
I'm not sure if there some wording that can clear that up in the first line.
- Line 28: this test should not need /othervm nor the explicit @compile
- There should be tests of align(n) with negative values.
- The for for for for structure doesn't follow the style guide.
On 8/29/18 10:00 AM, Jim Laskey wrote:
> Please review the code for String::align and String::indent at the link below.
> Includes a private version of String::isMultiline() which may be made into a public method at some future date
> Includes minor correctness clean up of StringLatin1.java, StringUTF16.java
> webrev: http://cr.openjdk.java.net/~jlaskey/8200434/webrev/index.html
> jbs: https://bugs.openjdk.java.net/browse/JDK-8200434
> — Jim
More information about the amber-dev