RFR: String.join(), StringJoiner additions
Alan.Bateman at oracle.com
Fri Apr 12 15:02:56 UTC 2013
On 11/04/2013 23:33, Jim Gish wrote:
> Please review
> These are changes that we made in lambda that we're now bringing into
> I've made a couple of additions - making StringJoiner final and adding
> a couple of constructors to set the emptyOutput chars.
One question on the bug numbers. Are you proposing to use all three? I
assume that at least 7175206 is not needed for the push to jdk8.
Just a few comments on String.join (which overall, it nice and simple).
The javadoc doesn't make it obvious that the elements can include null,
are you planning to make that clear?
A minor nit in the first example there is there is a space before the
It looks to be a bit odd to have one of the @see tags before the @throws
and the other after it.
In one method the @return wraps around without indenting, in the other
method the first @param is nicely indented but the second one is pushed
in by extra space.
The examples in the second String.join can probably use diamond. I don't
know how this looks in the generated javadoc but might be clearer to put
each of the strings.add on their own lines. There is also
inconsistencies with spaces and parentheses in these examples.
I see you proposing to put the test for String.join in
test/java/lang/StringTest.java but that is inconsistent with the current
organization. I realize this is TestNG test that is not in the unnamed
package but test/java/lang is just too high up in the tree for a
specific test. Also "StringTest" is a bit general a name given that it
is specific to String.join.
The usual copyright header for tests is the pure GPL header.
I plan to look at StringJoiner in more detail later.
More information about the core-libs-dev