RFR: 8017231: Add StringJoiner.merge

Henry Jen henry.jen at oracle.com
Wed Jul 3 16:57:23 UTC 2013

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?


More information about the core-libs-dev mailing list