RFR: String.join(), StringJoiner additions (original) (raw)

Alan Bateman Alan.Bateman at oracle.com
Thu Apr 18 11🔞42 UTC 2013


On 17/04/2013 22:49, Jim Gish wrote:

Here's an update: http://cr.openjdk.java.net/~jgish/Bugs-5015163-7172553/ <http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7172553/>

Jim StringJoiner looks much better now, good to see it reduced down to 2 simple constructors.

One thing that I didn't bring up in the previous round is that as all the parameters are CharSequences, then it probably should be made clear that the constructors and setEmptyValue method take copies. I'm not suggesting it would be logical to implement it otherwise but if someone using StringBuilder or mutable CharSequences implementation needs to be sure that StringJoiner will do the right thing if the char sequence is changed subsequently.

In the class description it reads:

"if the {@code emptyValue} parameter is supplied via the ..."

which is a bit confusing because a StringJoiner doesn't have a "emptyValue parameter". It might be clearer if changed to:

"if a default empty value is specified via the ..."

A minor comment on the wording in the constructors is that "Also" should probably be dropped from the second sentence.

The 3-arg constructor still specifies that it throws NPE if the emptyValue is null but there isn't an emptyValue parameter here.

In toString then I assume that you can avoid the +suffix when it is the empty string (which is going to very common).

You've addressed my previous comment on String.join not specifying how null behaves so this is clear now - thanks.

Also looks like you've moved the tests and extended the coverage for nulls - thanks.

So overall I think this is looking much better.

-Alan.



More information about the core-libs-dev mailing list