RFR: 8017231: Add StringJoiner.merge

[Henry Jen]

On Jul 3, 2013, at 9:09 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:

> On 03/07/2013 09:03, Henry Jen wrote:
>> Hi,
>> 
>> Please review a simple addition of StringJoiner.merge method. This is
>> useful to combine StringJoiners used in parallel doing joining works.
>> 
>> The webrev can be found at
>> http://cr.openjdk.java.net/~henryjen/ccc/8017231.0/webrev/
>> 
>> Also included is a little clean up for StringJoinerTest.
>> 
>> Cheers,
>> Henry
> Is this named "merge" to disambiguate it from "add"? Just wondering.
> 

I think it's a better reflection on the fact of "merge" of elements like collections, instead of feels like add(StringJoiner.toString()).

> A minor point but I think the wider javadoc tends to use "the given XYZ" when referring to parameters rather than the "the supplied XYZ".
> 
> Typo "nonempty" -> "non-empty".
> 

Will fix those.

> I assume you meant to name the local otherBuilder rather than an otherBuffer. Also is there any reason not to use append(CharSequence,int,int) here?
> 

Will fix the naming as well.

As for append, I don't think there is a particular reason.

It would be best if we can do append(char[], start, end), but I don't see there is any way to do that. I suspect append(CS, int, int) should be a little faster without increasing the count each time give a check of range.

I'll update it to use append(CharSequence, int, int).

> The test looks good to me, I guess an alternative would be to just add to the existing test so that we have one test for StringJoiner (it doesn't matter either way).
> 

I'll leave it out, I think it is easier to navigate that way.

Do we need another round for above updates?

Cheers,
Henry