RFR: 8066870 Add Readable::transferTo(Appendable)

[Brian Goetz]

Late to this thread, but ...

TL;DR: I think this method, as proposed, is fatally flawed; it should be 
a static method rather than a default method.


We need to be very, very careful about adding default methods like this 
to highly abstract interfaces like Readable (the -able suffix is often 
the giveaway that you're in murky waters wrt default methods).  There 
are many pitfalls one can run into here; the one that gets us here is 
that attempted overrides can easily and accidentally become overloads.

DIGRESSION -- A LESSON FROM 8

By way of background, we added Function.compose(...) in Java 8, when 
both functional interfaces and default methods were new, and this turned 
out to be a mistake.  This seemed a no-brainer (in j.l.f.Function), but 
didn't turn out so well.  We added:

intf Function<T,U> {
     <V> Function<T,V> compose(Function<U,V> g);
}

But note also that

intf BinaryOperator<T> extends Function<T,T> {
}

As such, binOp.compose(binOp) yields a Function<T,T>, not a BinOp<T>.  
Oops, that's not what we wanted!  If we try and override it:

intf BinaryOperator<T> extends Function<T,T> {
     BinaryOperator<T> compose(BinaryOperator<T> g);
}

we haven't really overridden compose(), but instead we've overloaded 
it.  And now we have potential ambiguities in overload resolution because

   f.compose(t -> t)

is compatible with both overrides of compose().  And since we don't have 
overloading on return types, we can't even use the return type to 
disambiguate:

   BinaryOperator<T> composed = f.compose(t -> t) // nope

The siren song here was the desire for fluency; this was just the wrong 
place to put a compose() method.  This method interacted with 
inheritance in two ways; it could be overridden, and it was also 
desirable for it to have distinguished behavior for distinguished 
subtypes of its argument types.  This was just too many degrees of 
freedom, and it painted us into a corner. Function was too general a 
place to put a default method like compose() that had such potential to 
interact with inheritance, but we were so excited about the prospects of 
saying "f.compose(g)" that we didn't think it through.

RETURNING TO THE CURRENT ISSUE

Now, while you could claim this is not an exact analogy (i.e., 
transferTo doesn't take a lambda, which was one ingredient of the 
problems with compose), the reality is that the higher up in the 
hierarchy you go, the more risky adding defaults is.  There still is a 
big interaction risk here.

In this case, the risk is that both Readable and Appendable are "too 
general."  In particular, it means that overriding in a subclass won't 
mean what you might think it means.  Think of the overridings that are 
actually likely to happen -- those where you know something more about 
both what kind of Readable and Appendable you've got.

Let's say that Reader does:

     class Reader {
         long transferTo(Writer out) { ... }
     }

This is a totally sensible thing to want to do, since Writer has more 
flexibility than Appendable, and so such an implementation could be 
better than the default.

The problem is: **this is not an override, but an overload**. Which 
means that

     Reader r = ...
     Writer.w = ...
     r.transferTo(w)

will not mean the same thing as

     Readable rr = r;
     Appendable ww = w;
     r.transferTo(ww);

Because InputStream.transferTo() is not an override of 
Readable.appendTo().  This is serious puzzler-bait!  Instance method 
dispatch is not supposed to depend on the static types involved, but 
that's sure what it looks like is happening here.

**IMO, this is a fatal API design error for this method**. Instead, 
Readable.transferTo() should probably be a static method, so that it 
does not give the illusion of being overridable.

I know default methods on highly abstract interfaces are very tempting, 
and the siren song of fluency calls to us, but we should be very, very 
careful about adding default methods to interfaces like Iterable or 
Readable -- essentially any of the Xxxable interfaces.

I think it is much better to make this a static method, and put concrete 
methods (with better implementations, that can take advantage of bulk 
write abilities, which Appendable lacks) on Reader as needed.  
(InputStream already has this method.)


On 11/17/2017 1:12 PM, Patrick Reinhart wrote:
> Hi Roger and Alan,
>
> I incorporated the latest feedback using version 1) from this latest post:
>
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-November/050004.html
>
>
> The actual webrev is here:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8066870/webrev.00
>
>
> -Patrick