RFR: 8017231: Add StringJoiner.merge

Henry Jen henry.jen at oracle.com
Wed Jul 3 17:14:02 UTC 2013


On Jul 3, 2013, at 9:57 AM, Henry Jen <henry.jen at oracle.com> wrote:

> 
> 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.
> 

I should said the range check is the reason. 

Cheers,
Henry


> 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).
> 




More information about the core-libs-dev mailing list