> Patrick, as soon as I get feedback from Alan and Paul I'll merge these
> changes into the current version.
>
> 1. I wonder if it makes sense to provide an additional method where client
> specifies CharBuffer to be used?
> Moreover if the first argument is CharSequence then we don't even need a
> buffer:
>
> /**
> * Reads all characters from a readable and appends them to an appendable.
> *
> * @param source the readable to read from ({@code source != null})
> * @param target the appendable to append to ({@code target != null})
> *
> * @return the number of characters copied
> *
> * @throws IOException if an I/O error occurs when reading or appending
> */
> public static long copy(Readable source, Appendable target)
> throws IOException {
> Objects.requireNonNull(source, "source");
> Objects.requireNonNull(target, "target");
>
> if (source instanceof CharSequence)
> return copy1((CharSequence) source, target);
>
> return copy0(source, target, CharBuffer.allocate(BUFFER_SIZE));
> }
>
> /**
> * Reads all characters from a readable and appends them to an appendable.
> *
> * @param source the readable to read from ({@code source != null})
> * @param target the appendable to append to ({@code target != null})
> * @param buffer the intermediate buffer to use
> * ({@code buffer != null && buffer.hasRemaining()
> * && !buffer.isReadOnly()})
> * @return the number of characters copied
> * @throws IOException if an I/O error occurs when reading or appending
> */
> public static long copy(Readable source, Appendable target,
> CharBuffer buffer) throws IOException {
> Objects.requireNonNull(source, "source");
> Objects.requireNonNull(target, "target");
> Objects.requireNonNull(buffer, "buffer");
> if (!buffer.hasRemaining())
> throw new IllegalArgumentException("Insufficient buffer size");
> if (buffer.isReadOnly())
> throw new IllegalArgumentException("buffer is read only");
> return copy0(source, target, buffer);
> }
>
> private static long copy1(CharSequence source, Appendable target)
> throws IOException {
> long copied = source.length();
> target.append(source);
> return copied;
> }
>
> private static long copy0(Readable source, Appendable target,
> CharBuffer buffer) throws IOException {
>
Seems reasonable to me. Will you add those changes additionally to mine then?
> 2. I think guts of the copy method could be changed to something like this:
>
> long copied = 0L;
> + int oldLim = buffer.limit();
>
> for (int read; (read = source.read(buffer)) > -1; copied += read) {
> buffer.flip();
> target.append(buffer, 0, read);
> + buffer.limit(oldLim);
> }
> return copied;
>
> Otherwise on each iteration you're potentially shrinking available buffer
> storage based on
> amount of data read into it (if I'm not mistaken). So each time you should
> restore the limit.
There I’m not that experienced either. I’m shamed about it, but never used
CharBuffer by myself so far… (my implementation could therefore be not
completely correct)
> 3. We should think of revisiting and including the only method from
> sun.misc.IOUtils. Removing
> the class and reparenting its dependencies.
That would be a followup task - on the way to authorship ;-)
-Patrick
