Feedback on the implementation of StringJoiner

[Roel Spilker]

Oh, and please remove the string concatenation from the constructor. Since
prefix and suffix are stored separately and setEmptyOutput does not accept
null values, a null-check on the field can be used to calculate the
emptyOutput when it is needed. I think the common case is that the
emptyOutput value is not read at all.


On Wed, Dec 12, 2012 at 9:27 PM, Roel Spilker <r.spilker at gmail.com> wrote:

> Well, a lot of code was removed, what is in my opinion always a good thing.
>
> Apart from the comments already mentioned, I have one other point. The
> JavaDoc of length(), charAt(int) and subSequence(int, int) still define
> their behavior as being equivalent to the same operation on the toString()
> result. However, the class isn't final. I don't think it is a promise that
> should be made, because it puts quite a burden on a subclass. My preferred
> solution would be to make the class final. However, I don't know if that is
> an option.
>
> Roel
>
>
>
> On Tue, Dec 11, 2012 at 9:54 AM, Stephen Colebourne <scolebourne at joda.org>wrote:
>
>> On 10 December 2012 16:26, Henry Jen <henry.jen at oracle.com> wrote:
>> > I am doing some changes to adapt your feedbacks and trying to have
>> minimum but enough APIs, latest proposed can be found at
>> >
>> > http://cr.openjdk.java.net/~henryjen/lambda/StringJoiner.1/webrev/
>>
>> StringJoiner
>> Line 61 example - missing bracket )
>>
>> First line of some method descriptions starts with lower case rather than
>> upper.
>>
>> Line 260 exception not listed in Javadoc
>>
>> As a general rule, I would try very hard to return a String, not a
>> CharSequence. Accepting a CharSequence on input is OK, but returning
>> it gives very few guarantees to callers. Think Postels law.
>>
>> Stephen
>>
>>
>