Yes, I meant the Reader.transferTo(Writer), and no I just did a quick search
over the JDK code base and I would to takle them as a second step. I would make
a couple of different patches for the various places due the fact that they are
in different parts of the JDK…
The question would be if I should proceed with the Reader.transferTo(Writer)
together with the static method on Readable.transferTo(Appendable)…
-Patrick
> Am 19.11.2017 um 23:14 schrieb Brian Goetz <brian.go...@oracle.com>:
>
> I presume you mean Reader.transferTo(Writer)? This seems fine, and you can
> have a much better implementation for Reader->Writer than for
> Readable->Appendable. (Do we get patches for those 30 places too?)
>
>
>
> On 11/19/2017 3:01 PM, Patrick Reinhart wrote:
>> Having a static method on Appendable instead of a default one seem
>> reasonable to me too.
>>
>> Would it be a problem to have an additional transferTo() method on Readable
>> as in your overload example though similar to IntputStream? My researches of
>> the JDK code base found at least 30 places where a simple transferTo on a
>> Reader could have been used to transfer all data to a Writer. Also some less
>> usages where a combination of Reader -> CharBuffer or CharBuffer ->
>> Appendable could have been used to. This analysis lead to the idea to have a
>> default method for those use cases instead may be use a separate static
>> method for Readable/Appendable and one on a Reader as on the InputStream
>>
>> -Patrick
>>
>>
>>> 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
>
