Am 19.11.2017 um 14:42 schrieb Brian Goetz <brian.go...@oracle.com>:
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
