JDK-8067661: transferTo proposal for Appendable

[Roger Riggs]

Hi Patrick,

A few comments:

Readable.java:
  67: + it may be worth mentioning that the input might not fit in 
output (as seen in the CharBuffer case)
Though I see we didn't call that out in the other transferTo description 
but here it is more likely that the output is bounded.

77: "The total amount will be added up by after the write method has 
been completed." should not be part
of the @implSpec.  It is untestable and unnecessary. If an exception 
occurs the value is lost.

96: "in order not having the extra overhead creating" -> "to avoid the 
extra overhead of"

X-Buffer.java.template:

1555: "form" -> "from"

1554: I would avoid most of the details in the @implSpec since it is 
requiring a specific use of CharBuffer methods.
That's fine as an @ImplNote but restricts the implementation too much as 
spec.
(others will disagree)

1565:  in the code, perhaps it should use an explicit 
RequireNonNull(out, "out") otherwise the NPE will be on put()/append().

1566: "insufficient space in this" refers to the source, not dest and it 
should only apply if out is a CharBuffer.
   Omit it or leave it more general;  that behavior is covered by the 
spec of the out Appendable.

1568: I don't see code to enforce:  "if out is this buffer"


On the tests; had you considered using TestNG;
it provides some good structure and is preferred (AFAIK) for new tests.

As for Randomness, it is useful to be explicit about the seed used in 
the particular run so it can be
replayed if necessary.  There is a testlibrary function to do that if 
you don't want to code-your-own.
(test/jdk/test/lib/RandomFactory::getRandom())

Thanks, Roger

On 11/9/2017 10:37 AM, Patrick Reinhart wrote:
> Finally did some review with Alan and integrated all into this webrev:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8067661/webrev.01 <http://cr.openjdk.java.net/~reinhapa/reviews/8067661/webrev.01>
>
> -Patrick
>
>> Am 08.11.2017 um 00:03 schrieb Brian Burkhalter <brian.burkhalter at oracle.com>:
>>
>> Hi Patrick,
>>
>> On Nov 7, 2017, at 3:01 PM, Patrick Reinhart <patrick at reini.net <mailto:patrick at reini.net>> wrote:
>>
>>> I integrated your changes into my current version which I will look into tomorrow with Alan at Devoxx. I think adding some implementation note on the default method implementation will be needed to review later on…
>> I concur.
>>
>>> I also made an override on the CharBuffer, where we need to add a enhanced documentation for the specialties around the CharBuffer implementations (BufferOverflowException/ReadOnlyBufferException), where a review of some native english speakers would help :-)
>> Sounds good!
>>
>> Thanks,
>>
>> Brian