On 16/04/2013 16:13, Jim Gish wrote:

On 04/15/2013 02:02 PM, Martin Buchholz wrote:

...

You are fiddling with the javadoc for getChars, which is an independent change. (I am also fiddling with getChars in another ongoing change). I don't think closing html tags for <li> are required in javadoc. If you are going to change the exception javadoc, then also change @exception to @throws.

The only reason I'm adding </li> is Alan insisted on it in a previous change I proposed :-)

I don't recall the full context but I have got confused by an early build of doclint where this was an issue. -Alan.

I'm going to rip out the </li> then. It's an unnecessary burden. Thanks Jim On 04/16/2013 06:50 PM, Mike Duigou wrote:

On Apr 16 2013, at 08:50 , Alan Bateman wrote:

...

On 16/04/2013 16:13, Jim Gish wrote:

...

On 04/15/2013 02:02 PM, Martin Buchholz wrote:

...

You are fiddling with the javadoc for getChars, which is an independent change. (I am also fiddling with getChars in another ongoing change). I don't think closing html tags for <li> are required in javadoc. If you are going to change the exception javadoc, then also change @exception to @throws.

The only reason I'm adding </li> is Alan insisted on it in a previous change I proposed :-)

I don't recall the full context but I have got confused by an early build of doclint where this was an issue. </li> is required by XHTML but not by HTML. doclint was too aggressive about this in early builds.

Having </li> does no harm but it's not required.

Mike

...

-Alan.

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com

On Apr 15, 2013, at 12:21 PM, Martin Buchholz <martinrb@google.com> wrote:

On Mon, Apr 15, 2013 at 11:31 AM, Martin Buchholz <martinrb@google.com>wrote:

...

OTOH, I'm guessing you are trying to improve the performance of operations like List.toString. More efficient (single copy char[]) would be to collect all the sub-CharSequences in a CharSequence[], pre-compute the final length of the char[], allocate an array of exactly the required length, and create the final string directly from that using the package-private constructor (but in the unlikely event that a subsequence changed in size while concat'ing, be prepared to resize the array).

Proceeding further along this train of thought, I might start with AbstractCollection.toString() (and similar methods) and attempt to make it maximally efficient. Maybe add a method to JavaLangAccess to make a String directly from a perfectly sized array (as needed elsewhere?). Maybe create a StringBuilder-like class that works better for typical use cases?

For what it's worth, a patch that I contributed and Mike (and others) then rewrote contains this functionality already: http://cr.openjdk.java.net/~mduigou/JDK-8006627/2/webrev/src/share/classes/s... It's not merged yet though.

On Apr 15 2013, at 13:03 , Steven Schlansker wrote:

On Apr 15, 2013, at 12:21 PM, Martin Buchholz <martinrb@google.com> wrote:

...

On Mon, Apr 15, 2013 at 11:31 AM, Martin Buchholz <martinrb@google.com>wrote:

...

OTOH, I'm guessing you are trying to improve the performance of operations like List.toString. More efficient (single copy char[]) would be to collect all the sub-CharSequences in a CharSequence[], pre-compute the final length of the char[], allocate an array of exactly the required length, and create the final string directly from that using the package-private constructor (but in the unlikely event that a subsequence changed in size while concat'ing, be prepared to resize the array).

Proceeding further along this train of thought, I might start with AbstractCollection.toString() (and similar methods) and attempt to make it maximally efficient. Maybe add a method to JavaLangAccess to make a String directly from a perfectly sized array (as needed elsewhere?). Maybe create a StringBuilder-like class that works better for typical use cases?

For what it's worth, a patch that I contributed and Mike (and others) then rewrote contains this functionality already:

http://cr.openjdk.java.net/~mduigou/JDK-8006627/2/webrev/src/share/classes/s...

It's not merged yet though.

It is not forgotten. :-)

Here's an update: http://cr.openjdk.java.net/~jgish/Bugs-5015163-7172553/ <http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7172553/> Jim On 04/17/2013 03:15 PM, Mike Duigou wrote:

String::

line 1253: This should use {@code } rather than <code></code>. I think regular spaces are OK as well.   seems inappropriate.

lines 2425/2467: elements may not be null either.

I can tell you (or maybe it's just me) are itching to change :

for (CharSequence cs: elements) { 2477 joiner.add(cs); 2478 }

to:

elements.forEach(joiner::add);

StringJoiner::

- <blockquote> isn't needed around <pre> as it's already a <div> you probably mean to do

<pre> {@code ... }</pre>

for code samples.

- It would be nice if the empty output generation in three arg constructor could be suppressed unless needed. Perhaps a special (not null please!) sentinel value?

- Four arg constructor doesn't include emptyOutput in @throws NPE

On Apr 11 2013, at 15:33 , Jim Gish wrote:

...

Please review http://cr.openjdk.java.net/~jgish/Bugs-5015163-7175206-7172553/ <http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7175206-7172553/>

These are changes that we made in lambda that we're now bringing into JDK8.

I've made a couple of additions - making StringJoiner final and adding a couple of constructors to set the emptyOutput chars.

Thanks, Jim

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com

I'm open to this, but am interested in what others have to say, especially as it relates to other lambda features coming in. Bear in mind that this is at least the third major round of reviews for these changes, the first round being a year ago on lambda-dev, when I first submitted them, and then they were distilled some more and greatly simplified by Henry Jen. Thanks, Jim On 04/17/2013 06:07 PM, Martin Buchholz wrote:

I'm still wondering about whether a joiner utility should support a prefix and suffix. The obvious uses for this are collection class toString methods, but we already know that we can and should implement those with a single precise char[] construction, so should not use StringJoiner, or at least not this StringJoiner implementation. And if we're just talking about pure convenience, it's hard to beat

"[" + String.join(...) + "]"

On Wed, Apr 17, 2013 at 2:49 PM, Jim Gish <jim.gish@oracle.com <mailto:jim.gish@oracle.com>> wrote:

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

Jim

On 04/17/2013 03:15 PM, Mike Duigou wrote:

String::

line 1253: This should use {@code } rather than <code></code>. I think regular spaces are OK as well.   seems inappropriate.

lines 2425/2467: elements may not be null either.

I can tell you (or maybe it's just me) are itching to change :

for (CharSequence cs: elements) { 2477 joiner.add(cs); 2478 }

to:

elements.forEach(joiner::add);

StringJoiner::

- <blockquote> isn't needed around <pre> as it's already a <div> you probably mean to do

<pre> {@code ... }</pre>

for code samples.

- It would be nice if the empty output generation in three arg constructor could be suppressed unless needed. Perhaps a special (not null please!) sentinel value?

- Four arg constructor doesn't include emptyOutput in @throws NPE

On Apr 11 2013, at 15:33 , Jim Gish wrote:

Please review http://cr.openjdk.java.net/~jgish/Bugs-5015163-7175206-7172553/ <http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7175206-7172553/> <http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7175206-7172553/>

These are changes that we made in lambda that we're now bringing into JDK8.

I've made a couple of additions - making StringJoiner final and adding a couple of constructors to set the emptyOutput chars.

Thanks, Jim

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 <tel:%2B1.781.442.0304> Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com <mailto:jim.gish@oracle.com>

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 <tel:%2B1.781.442.0304> Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com <mailto:jim.gish@oracle.com>

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com

To be concrete, here's a proposed toString method : http://cr.openjdk.java.net/~martin/webrevs/openjdk8/toString/ which is pretty good as it stands, but even better once it gets to use the no-copy String constructor.

Hi, I'm wondering, that StringJoiner has some logic for pre/suffix, but nothing to loop the elements themselves :-( To me, StringJoiner is a useless complicated box around StringBuilder, and imagine, someone needs thread-safety. It also slows down performance, as it needs additional instances and additional class to be loaded (critical at VM startup). Instead please add to StringBuilder and StringBuffer: append(CharSequence... elements); append(char delimiter, CharSequence... elements); append(char delimiter, Iterable<? extends CharSequence> elements); cut(int len); // removes len chars at the end of the sequence optional: append(CharSequence delimiter, CharSequence... elements); append(CharSequence delimiter, Iterable<? extends CharSequence> elements); For performance reasons, append should always append the trailing delimeter, which could be cut at the end. It's questionable, if class string needs a static (=no relation to an existing string in contrast to non-static split()) join method, as it seduces to "[" + String.join(...) + "]" which needs some effort from javac side to optimize to a single StringBuilder task. IMO we better had StringBuilder.join(...), so javac could easily optimize to: new StringBuilder().append('[').append(',', someStrings).cut(1).append(']').toString() -Ulf Am 18.04.2013 00:07, schrieb Martin Buchholz:

I'm still wondering about whether a joiner utility should support a prefix and suffix. The obvious uses for this are collection class toString methods, but we already know that we can and should implement those with a single precise char[] construction, so should not use StringJoiner, or at least not this StringJoiner implementation. And if we're just talking about pure convenience, it's hard to beat

"[" + String.join(...) + "]"

On Wed, Apr 17, 2013 at 2:49 PM, Jim Gish <jim.gish@oracle.com> wrote:

...

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

On 04/17/2013 03:15 PM, Mike Duigou wrote:

...

String::

line 1253: This should use {@code } rather than <code></code>. I think regular spaces are OK as well.   seems inappropriate.

lines 2425/2467: elements may not be null either.

I can tell you (or maybe it's just me) are itching to change :

for (CharSequence cs: elements) { 2477 joiner.add(cs); 2478 }

to:

elements.forEach(joiner::add);

StringJoiner::

- <blockquote> isn't needed around <pre> as it's already a <div> you probably mean to do

<pre> {@code ... }</pre>

for code samples.

- It would be nice if the empty output generation in three arg constructor could be suppressed unless needed. Perhaps a special (not null please!) sentinel value?

- Four arg constructor doesn't include emptyOutput in @throws NPE

On Apr 11 2013, at 15:33 , Jim Gish wrote:

Please review http://cr.openjdk.java.net/~**jgish/Bugs-5015163-7175206-*

...

*7172553/<http://cr.openjdk.java.net/~jgish/Bugs-5015163-7175206-7172553/><; http://cr.openjdk.java.net/%**7Ejgish/Bugs-5015163-7175206-**7172553/<http://cr.openjdk.java.net/%7Ejgish/Bugs-5015163-7175206-7172553/> These are changes that we made in lambda that we're now bringing into JDK8.

I've made a couple of additions - making StringJoiner final and adding a couple of constructors to set the emptyOutput chars.

Thanks, Jim

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com

Garbled javadoc: 41 * method will, by default, return {@code prefix+{@code suffix}}. However, if

Am 18.04.2013 19:37, schrieb Jim Gish:

On 04/18/2013 08:49 AM, Ulf Zibis wrote:

...

Hi,

I'm wondering, that StringJoiner has some logic for pre/suffix, but nothing to loop the elements themselves :-(

To me, StringJoiner is a useless complicated box around StringBuilder, and imagine, someone needs thread-safety. It also slows down performance, as it needs additional instances and additional class to be loaded (critical at VM startup).

Instead please add to StringBuilder and StringBuffer: append(CharSequence... elements); append(char delimiter, CharSequence... elements); append(char delimiter, Iterable<? extends CharSequence> elements); cut(int len); // removes len chars at the end of the sequence optional: append(CharSequence delimiter, CharSequence... elements); append(CharSequence delimiter, Iterable<? extends CharSequence> elements); I started off with something similar, but it was stripped out when Henry did the performance improvements.

Hm, I have no idea, how above suggestions should prevent performance improvements. Can you help me?

Given that most people feel that this is going to be put to heavy-weight usage, it doesn't seem to merit too much emphasis on performance or complicating the implementation at this point.

Your implementation has 1 class with 7 methods 2 additional methods in String To cover the same functionality, above approach basically only needs 2 additional methods in StringBuilder, has better performance, so what is complicated on that? @Martin: What is your opinion? Thanks, -Ulf

...

For performance reasons, append should always append the trailing delimeter, which could be cut at the end.

It's questionable, if class string needs a static (=no relation to an existing string in contrast to non-static split()) join method, as it seduces to "[" + String.join(...) + "]" which needs some effort from javac side to optimize to a single StringBuilder task. IMO we better had StringBuilder.join(...), so javac could easily optimize to: new StringBuilder().append('[').append(',', someStrings).cut(1).append(']').toString()

-Ulf

Am 18.04.2013 00:07, schrieb Martin Buchholz:

...

I'm still wondering about whether a joiner utility should support a prefix and suffix. The obvious uses for this are collection class toString methods, but we already know that we can and should implement those with a single precise char[] construction, so should not use StringJoiner, or at least not this StringJoiner implementation. And if we're just talking about pure convenience, it's hard to beat

"[" + String.join(...) + "]"

On Wed, Apr 17, 2013 at 2:49 PM, Jim Gish <jim.gish@oracle.com> wrote:

...

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

Jim

Sorry I am not receiving emails from core-lib-dev earlier. What I did was to remove things we considered not absolutely required, which left us only a StringJoiner class and two String.join methods. StringJoiner was intended to be used with Stream as a collector, thus prefix, and suffix is desired like Collection.toString(). Cheers, Henry On Apr 19, 2013, at 9:54 AM, Jim Gish <jim.gish@oracle.com> wrote:

Hi Henry, Can you please comment on the simplifications you did ?

Thanks, Jim

On 04/18/2013 07:38 PM, Ulf Zibis wrote:

...

Am 18.04.2013 19:37, schrieb Jim Gish:

...

On 04/18/2013 08:49 AM, Ulf Zibis wrote:

...

Hi,

I'm wondering, that StringJoiner has some logic for pre/suffix, but nothing to loop the elements themselves :-(

To me, StringJoiner is a useless complicated box around StringBuilder, and imagine, someone needs thread-safety. It also slows down performance, as it needs additional instances and additional class to be loaded (critical at VM startup).

Instead please add to StringBuilder and StringBuffer: append(CharSequence... elements); append(char delimiter, CharSequence... elements); append(char delimiter, Iterable<? extends CharSequence> elements); cut(int len); // removes len chars at the end of the sequence optional: append(CharSequence delimiter, CharSequence... elements); append(CharSequence delimiter, Iterable<? extends CharSequence> elements); I started off with something similar, but it was stripped out when Henry did the performance improvements.

Hm, I have no idea, how above suggestions should prevent performance improvements. Can you help me?

...

Given that most people feel that this is going to be put to heavy-weight usage, it doesn't seem to merit too much emphasis on performance or complicating the implementation at this point.

Your implementation has 1 class with 7 methods 2 additional methods in String To cover the same functionality, above approach basically only needs 2 additional methods in StringBuilder, has better performance, so what is complicated on that?

@Martin: What is your opinion?

Thanks,

-Ulf

...

...

For performance reasons, append should always append the trailing delimeter, which could be cut at the end.

It's questionable, if class string needs a static (=no relation to an existing string in contrast to non-static split()) join method, as it seduces to "[" + String.join(...) + "]" which needs some effort from javac side to optimize to a single StringBuilder task. IMO we better had StringBuilder.join(...), so javac could easily optimize to: new StringBuilder().append('[').append(',', someStrings).cut(1).append(']').toString()

-Ulf

Am 18.04.2013 00:07, schrieb Martin Buchholz:

...

I'm still wondering about whether a joiner utility should support a prefix and suffix. The obvious uses for this are collection class toString methods, but we already know that we can and should implement those with a single precise char[] construction, so should not use StringJoiner, or at least not this StringJoiner implementation. And if we're just talking about pure convenience, it's hard to beat

"[" + String.join(...) + "]"

On Wed, Apr 17, 2013 at 2:49 PM, Jim Gish <jim.gish@oracle.com> wrote:

...

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

-- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish@oracle.com

Hi, I don't think it's worth to add the braces at lines 2359..2360. Please swap lines 2740 <-> 2739. -Ulf Am 18.04.2013 03:20, schrieb Roger Riggs:

Hi,

Can I suggest that the StringJoiner.toString() method explicitly append the suffix to the existing StringBuilder.

152 return (value != null ? value.toString() + suffix : emptyValue);

Currently it will go to the trouble of creating a String from the builder and then transparently create another StringBuilder to do the concatenation.

152 return (value != null ? value.append(suffix).toString() : emptyValue);

or something similar.

The overhead of StringJoiner supporting prefix and suffix is lower than doing it separately in terms of object allocations and garbage and all places that would need write their own code to do the concatenation.

Roger

On 4/17/13 5:49 PM, 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

On Thu, Apr 18, 2013 at 10:34 AM, Jim Gish <jim.gish@oracle.com> wrote:

That was a nice idea, but you don't want to change the value when you do toString(). Otherwise, if you subsequently add a new element, you're hosed because you've already added on the suffix.

You can cheaply save the current length, append the suffix, call toString, and reset the length back to the old value to avoid the overhead of String "+".

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 <i>empty value</i> 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.

On Jul 23 2013, at 12:43 , ph wrote:

didn't see delimiter collision mentioned anywhere - are you really offering an interface that only joins a list of entries without escaping or quoting or even checking for separators (or escape or quote sequences) that might occur in the entries?

Correct. StringJoiner makes no effort to address escaping/quoting. Doing so would add a lot of complexity that would not useful interesting to most users and probably still wouldn't satisfy everyone. If you wish some form of escaping or quoting you can pre-processed entries however you like before joining them. Mike

On Jul 23, 2013, at 1:37 PM, ph <philip.hodges@bluewin.ch> wrote:

The omission of that very complexity is a copious source of bugs that breaks applications in production, and is extremely useful and interesting to the black hat community. It should not be possible to call a joiner without escaping or quoting or catching an exception or explicitly waiving the complexity. Of course there will still be some users who roll their own, blissfully unaware of the existence of StringJoiner. But for those who do discover it, the important value to add is not convenience, but safety. Maybe you could even retrofit it to Arrays.toString(), which is exactly such a toy interface only good for things like logging where delimiter collision doesn't usually matter. At the very least can you please cover it in the examples so that people can't completely overlook it?

Hi, If you are expecting to read the data back in a structured manner, I would expect you to use a proper structured format e.g. CSV or JSON, not a simple string joiner. String joining seems to work the same in every language I see (Python, Ruby, Javascript at least) and given that it takes arbitrary (potentially multi-character) delimiters I'm not even sure what behavior you'd expect that could make sense with any given delimiter. The current behavior is similar to other languages, simple to understand, and seems to not be "surprising" to most people. Why would you not write your data out as e.g. JSON? Then all of the tricky cases are handled correctly for you. Best, Steven

On 07/23/2013 11:57 PM, ph wrote:

Nothing against a "simple" joiner that forces users to catch or rule out a DelimiterCollisionException, and realise that simple is not good enough. Everything against a stupid joiner that silently assists users in creating bugs.

"Seems to work" means not tested thoroughly. People will be tempted into using this joiner to write csv files, and of course it will break as soon as a comma or semicolon delimiter turns up in a text field. They will use it to put br line break elements into HTML files, without checking for HTML syntax characters in each line. They will use it to separate SQL column values with commas, instead of binding them, or calling mysql_real_escape_string, which is a pain to use because it needs a database connection at runtime to find out exactly which characters need escaping depending on which SQL mode is configured.

By all means use a "proper structured format", so long as the delimiting is properly specified and the parser and generator are fully tested. That might well be the case for JSON and XML, but it typically is not for CSV and HTML or properties files.

Just because other languages may happen to have something similar does not automatically make it easy to use safely.

The current behaviour encourages SQL injection. How hard is that to understand? There are some very good features coming in this release, but this joiner is not one of them; it is actually dangerous.

Java is not PHP, data are represented by objects not Strings so escaping (if needed*) is done by the API not by the users. Rémi * sql drivers in Java often use binary protocols not textual one.

On Aug 30 2013, at 23:40 , Philip Hodges wrote:

...

Why is there such a bandwagon rolling for this "convenience" feature?

Perhaps others just don't agree with you. The choice of functionality offered in the JDK 8 StringJoiner was not arbitrary or haphazardly chosen. If it doesn't meet your particular needs, I am sorry to hear that. Our belief as the proposers is that it offers a reasonable mix of functionality and simplicity to cover useful number of requirements. Our intent was certainly not to create the kitchen magician [1] of string joiners that attempted to incorporate every conceivable option. In particular your concerns about escaping are out of scope for the joiner in part because Collection<String> strings; String concat = strings.stream().map(MyUtils::escape).collect(Collections.joining(",")); seems like an adequate and flexible solution to most people.

We already have joiners in apache commons and guava.

This is the reason that StringJoiner was considered for JDK--that many others were "rolling their own".

At first I thought they were cool. Then I tried to use them in anger. And was forced to roll my own.

I would encourage you to share your implementation or the javadocs as grist for discussion. An actual alternative is the *only* thing that is likely to be persuasive.

That can't be right.

Sometimes it is. The JDK doesn't make any attempt to satisfy everyone (or anyone). We all end up writing lots of non-business logic utility functions to bridge gaps in what JDK offers. This is normal. And, if it turns out to be something lots of people need then it's entirely possible that it could be added to the JDK. Mike [1] https://www.youtube.com/watch?v=cGVG9xLHb84

A more elaborate offically blessed feature that only does half the job is worse than useless. Without the extra complex ability to detect or avoid collisions it is neither "nice", nor a "Good Thing".

Phil

http://www.techempower.com/blog/2013/03/26/everything-about-java-8/

"StringJoiner and String.join(...) are long, long overdue. They are so long overdue that the vast majority of Java developers likely have already written or have found utilities for joining strings, but it is nice for the JDK to finally provide this itself. Everyone has encountered situations where joining strings is required, and it is a Good Thing™ that we can now express that through a standard API that every Java developer (eventually) will know. "

On 2013-07-23 22:09, Mike Duigou wrote:

...

On Jul 23 2013, at 12:43 , ph wrote:

...

didn't see delimiter collision mentioned anywhere - are you really offering an interface that only joins a list of entries without escaping or quoting or even checking for separators (or escape or quote sequences) that might occur in the entries?

Correct. StringJoiner makes no effort to address escaping/quoting. Doing so would add a lot of complexity that would not useful interesting to most users and probably still wouldn't satisfy everyone.

...

If you wish some form of escaping or quoting you can pre-processed entries however you like before joining them.

Mike

Please please please drop StringJoiner from Java 1.8 before it is too late. It is not needed. It does not add value. It is an embarrassment. We did without it for years. It is not long long overdue. We do not need it now. It does not need to be in the very first Java 1.8 release. Try leaving it out, and see if people even complain. Most people won't even notice it, let alone be ready to misuse it straight away. String.join is slower than Arrays.toString() or Collections.toString(). Test failed. String.join(delimiter, iterable) needs extra code to call toString() if elements are not already CharSequence. Convenience? Test failed. StringJoiner does not force or even remind the developer to consider delimiter collisions. Test failed. It is a perfect complement for split: the hard to use interface that can't split the joined input reliably because it might have too many delimiters. It will be the newest star on java antipattern web sites. A good place would be just after the item about not "Assembling XML with String operations" because the text parts might contain special characters. Even if it ever did appear that StringJoiner and String.join did what we wanted, I would still much rather roll my own than use the new StringJoiner in the library. Then I could set the initial capacity of the StringBuilder, skip the defensive copies and null checks, and above all, introduce some extra mandatory method call to force the developer to tell the compiler what is being done about delimiter collisions, even if it is to add an assert that there are no delimiters in the added CharSequences, or to waive that check because numbers never never contain comma delimiters (hold on ... better check the locale to see if comma is used as a decimal point or grouping character after all). I came across this non-justification [with my additions] on a blog: "StringJoiner and String.join(...) are long, long overdue. They are so long overdue that the vast majority of Java developers likely have already written or have found [or have had to mend] utilities for joining strings [to make an unsplittable delimiter collision], but it is nice [or at least well-meant] for the JDK to finally provide this itself. Everyone has encountered situations where joining strings is required [and they forgot all about escaping, or should have used a PreparedStatement instead], and it is a Good Thing™ that we can now express that [bug opportunity] through a standard API that every [no, just some] Java developer (eventually) will know [of, but not actually use, and even if they do they will need an IDE to tell them that the prefix comes *after* the delimiter]."

To quote Joshua Bloch on good API design: 'When in doubt leave it out' (or 'You can always add, but you can never remove')

On 31 Aug 2013, at 23:13, Mike Duigou <mike.duigou@oracle.com> wrote:

On Aug 30 2013, at 23:40 , Philip Hodges wrote:

...

...

Why is there such a bandwagon rolling for this "convenience" feature?

Perhaps others just don't agree with you. The choice of functionality offered in the JDK 8 StringJoiner was not arbitrary or haphazardly chosen. If it doesn't meet your particular needs, I am sorry to hear that. Our belief as the proposers is that it offers a reasonable mix of functionality and simplicity to cover useful number of requirements. Our intent was certainly not to create the kitchen magician [1] of string joiners that attempted to incorporate every conceivable option. In particular your concerns about escaping are out of scope for the joiner in part because

Collection<String> strings;

String concat = strings.stream().map(MyUtils::escape).collect(Collections.joining(","));

seems like an adequate and flexible solution to most people.

...

We already have joiners in apache commons and guava.

This is the reason that StringJoiner was considered for JDK--that many others were "rolling their own".

...

At first I thought they were cool. Then I tried to use them in anger. And was forced to roll my own.

I would encourage you to share your implementation or the javadocs as grist for discussion. An actual alternative is the *only* thing that is likely to be persuasive.

...

That can't be right.

Sometimes it is. The JDK doesn't make any attempt to satisfy everyone (or anyone). We all end up writing lots of non-business logic utility functions to bridge gaps in what JDK offers. This is normal. And, if it turns out to be something lots of people need then it's entirely possible that it could be added to the JDK.

Mike

[1] https://www.youtube.com/watch?v=cGVG9xLHb84

...

A more elaborate offically blessed feature that only does half the job is worse than useless. Without the extra complex ability to detect or avoid collisions it is neither "nice", nor a "Good Thing".

Phil

http://www.techempower.com/blog/2013/03/26/everything-about-java-8/

"StringJoiner and String.join(...) are long, long overdue. They are so long overdue that the vast majority of Java developers likely have already written or have found utilities for joining strings, but it is nice for the JDK to finally provide this itself. Everyone has encountered situations where joining strings is required, and it is a Good Thing™ that we can now express that through a standard API that every Java developer (eventually) will know. "

On 2013-07-23 22:09, Mike Duigou wrote:

...

On Jul 23 2013, at 12:43 , ph wrote:

...

didn't see delimiter collision mentioned anywhere - are you really offering an interface that only joins a list of entries without escaping or quoting or even checking for separators (or escape or quote sequences) that might occur in the entries?

Correct. StringJoiner makes no effort to address escaping/quoting. Doing so would add a lot of complexity that would not useful interesting to most users and probably still wouldn't satisfy everyone.

...

If you wish some form of escaping or quoting you can pre-processed entries however you like before joining them.

Mike

I did not predict that it would be a "sprintf". It's not going to be consistently misused anything like so frequently. I compared it to the unescaped XML generation antipattern. I have not seen any technical justifications whatsoever so far, just inexplicable enthusiasm. It is like giving young drivers a faster car with no seat belts. The trouble is, unlike in Google guava, this is the JDK, you can't just drop flawed beta interfaces two versions later. All you can do is add them to static review tool blacklists and deprecate them to cause a warning at compile time, and hope that people will give them a wide berth. Even if you hide them in an unoffical sun.com package. By the way, I'm looking forward to the proprietary Base64 being changed to simply call the new official one. You can't even modify an equals method in an undocumented reflection implementation in 1.7.0.51 without breaking production applications. Wow. I am raising doubt and you are ignoring it. Don't you have the guts to say whoa, stop the bandwagon, there could just be a real problem with this, it really will make it easier to create bugs without any warning from the compiler and without making anyone's code better *in any way at all*? I am picking on String.join precisely because I have seen way too many delimiter collision bugs, not just in Java but in several other languages, and because this interface perpetuates a real world problem by preventing future interface changes to detect them. [I am not always a doomsayer. For example, I am not picking on JSR310 because Date and Calendar and SimpleDateFormat are well known disaster areas and the people working on their replacement have a deep understanding of the issues, have really taken their time, and nothing whatsoever that I have read about it jumped out at me and said this is *wrong*. It might not even be completely perfect. But I am confident it will be so much better than what we have now, and it's a shame that I won't have time to migrate existing apps to it.] The counterproposal, no, proposal refinement, wafting around inside my head, is to somehow compel programmers to make just one more method call before that final toString(). It will be difficult to find good names, especially ones that will be understood by programmers for whom English is not a first language. Something like: String.join(delimiter, joinables).assertNoUnescapedDelimiters().toString(); String.join(delimiter, joinables).neverMindDelimiterCollisions().toString(); String.join(delimiter, joinables).promiseNoUnescapedDelimiters().toString(); String.join(delimiter, joinables).escapeDelimiters(escapeChar).toString(); String.join(delimiter, joinables).quoteElements(quoteChar).toString(); The vital thing is that String.join has to return an unjoinable, that needs an adapter method to make it safely joinable. If you get that right, then we can forgive this first shot being a little slow, and enjoy the triumph of CharSequence over immutability. Yes, ultimately the goal should be to add full support for at least the most popular csv generation recipes. I'm really sorry I couldn't carry on arguing the case before August. As a minority, I only have one person's quota of energy. I will try to get some more people to look at it. On 27 Jan 2014, at 18:44, Mike Duigou <mike.duigou@oracle.com> wrote:

On Jan 26 2014, at 17:12 , Philip Hodges <philip.hodges@bluewin.ch> wrote:

...

Please please please drop StringJoiner from Java 1.8 before it is too late.

It is well past "too late". The API has been frozen since August for all but the most exceptional cases.

...

...

...

At first I thought they were cool. Then I tried to use them in anger. And was forced to roll my own.

I would encourage you to share your implementation or the javadocs as grist for discussion. An actual alternative is the *only* thing that is likely to be persuasive.

You seem to be very much in the minority on this issue by the silence from other responders on this list and elsewhere. The concerns you've highlighted with regard to delimiter management, while certainly valid, have not been of sufficient concern to suggest a change in the proposal. The prediction that using StringJoiner will become Java's sprintf seems unlikely to be be true. The reception we've seen thus far for StringJoiner has been otherwise exclusively enthusiastic and positive.

Alternatives, refinements and counterproposals are always welcome in the discussion. Doomsaying unfortunately adds little.

Mike

String is probably the most commonly used class in the whole of Java. Please please please do not pollute it with this dangerous new method. Your Java-is-cool example is ridiculously trivial, of no practical use whatsoever, and absolutely *not* cool. How about a proper test where you have to double up the delimiter if it occurs in an element: assertEquals("'Java','isn''t','perfect'", "'"+String.join("','", javaIsNotPerfect)+"'"); I now have a working proof of concept, where newJoiner returns an UnsplittableJoiner which requires a delimiter handler to convert it back into a Joiner. Actually my joiner is an ObjectJoiner, unlike your StringJoiner that is just a CharSequenceJoiner, unlike AbstractCollections.toString and Arrays.toString which can join any elements that have a toString method. Please have the guts to pull String.join from the release now, before it becomes an antipattern, and you have to deprecate it. And tell everybody exactly why. Seriously. On 27 Jan 2014, at 21:31, Philip Hodges <philip.hodges@bluewin.ch> wrote:

I did not predict that it would be a "sprintf". It's not going to be consistently misused anything like so frequently. I compared it to the unescaped XML generation antipattern.

I have not seen any technical justifications whatsoever so far, just inexplicable enthusiasm.

It is like giving young drivers a faster car with no seat belts.

The trouble is, unlike in Google guava, this is the JDK, you can't just drop flawed beta interfaces two versions later. All you can do is add them to static review tool blacklists and deprecate them to cause a warning at compile time, and hope that people will give them a wide berth. Even if you hide them in an unoffical sun.com package. By the way, I'm looking forward to the proprietary Base64 being changed to simply call the new official one. You can't even modify an equals method in an undocumented reflection implementation in 1.7.0.51 without breaking production applications. Wow.

I am raising doubt and you are ignoring it. Don't you have the guts to say whoa, stop the bandwagon, there could just be a real problem with this, it really will make it easier to create bugs without any warning from the compiler and without making anyone's code better *in any way at all*?

I am picking on String.join precisely because I have seen way too many delimiter collision bugs, not just in Java but in several other languages, and because this interface perpetuates a real world problem by preventing future interface changes to detect them.

[I am not always a doomsayer. For example, I am not picking on JSR310 because Date and Calendar and SimpleDateFormat are well known disaster areas and the people working on their replacement have a deep understanding of the issues, have really taken their time, and nothing whatsoever that I have read about it jumped out at me and said this is *wrong*. It might not even be completely perfect. But I am confident it will be so much better than what we have now, and it's a shame that I won't have time to migrate existing apps to it.]

The counterproposal, no, proposal refinement, wafting around inside my head, is to somehow compel programmers to make just one more method call before that final toString(). It will be difficult to find good names, especially ones that will be understood by programmers for whom English is not a first language. Something like:

String.join(delimiter, joinables).assertNoUnescapedDelimiters().toString(); String.join(delimiter, joinables).neverMindDelimiterCollisions().toString(); String.join(delimiter, joinables).promiseNoUnescapedDelimiters().toString(); String.join(delimiter, joinables).escapeDelimiters(escapeChar).toString(); String.join(delimiter, joinables).quoteElements(quoteChar).toString();

The vital thing is that String.join has to return an unjoinable, that needs an adapter method to make it safely joinable. If you get that right, then we can forgive this first shot being a little slow, and enjoy the triumph of CharSequence over immutability.

Yes, ultimately the goal should be to add full support for at least the most popular csv generation recipes.

I'm really sorry I couldn't carry on arguing the case before August. As a minority, I only have one person's quota of energy. I will try to get some more people to look at it.

On 27 Jan 2014, at 18:44, Mike Duigou <mike.duigou@oracle.com> wrote:

...

On Jan 26 2014, at 17:12 , Philip Hodges <philip.hodges@bluewin.ch> wrote:

...

Please please please drop StringJoiner from Java 1.8 before it is too late.

It is well past "too late". The API has been frozen since August for all but the most exceptional cases.

...

...

...

At first I thought they were cool. Then I tried to use them in anger. And was forced to roll my own.

I would encourage you to share your implementation or the javadocs as grist for discussion. An actual alternative is the *only* thing that is likely to be persuasive.

You seem to be very much in the minority on this issue by the silence from other responders on this list and elsewhere. The concerns you've highlighted with regard to delimiter management, while certainly valid, have not been of sufficient concern to suggest a change in the proposal. The prediction that using StringJoiner will become Java's sprintf seems unlikely to be be true. The reception we've seen thus far for StringJoiner has been otherwise exclusively enthusiastic and positive.

Alternatives, refinements and counterproposals are always welcome in the discussion. Doomsaying unfortunately adds little.

Mike