RFR - JDK-8200434 - String::align, String::indent (code review)
Roger Riggs
roger.riggs at oracle.com
Wed Sep 5 20:07:52 UTC 2018
Hi Jim,
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.
AlignIndent.java:
- 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.
Thanks, Roger
(Sorry for the duplicates, I missed core-libs the first time)
On 8/29/18 10:00 AM, Jim Laskey wrote:
> Please review the code for String::align and String::indent at the link below.
>
> Notes:
> 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
>
> Cheers,
>
> — Jim
>
More information about the amber-dev
mailing list