StringJoiner: detect or fix delimiter collision?

Philip Hodges philip.hodges at bluewin.ch
Wed Feb 5 20:29:30 UTC 2014


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 at 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 at oracle.com> wrote:
> 
>> 
>> On Jan 26 2014, at 17:12 , Philip Hodges <philip.hodges at 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
> 




More information about the core-libs-dev mailing list