Hannes,

Thanks.

I was not planning to update the use of the methods everywhere, unless we can find an IDE refactoring to help us.

In many of the cases I have looked at, I want to do more significant (but local) restructuring and rearrangement, and when we get to do that, on a case-by-case basis, being able to use the chained methods will be really nice.

I also think that chained method calls will play nicely into the proposed new "builders as Content" paradigm and upcoming changeset.

I'll update the comments on Table before pushing.

-- Jon

On 3/6/20 9:08 AM, Hannes Wallnöfer wrote:
Hi Jon,

Looks good.

Out of curiosity, do you plan to update uses of these methods in a separate 
changeset?

A very minor nitpick: in Table.java there are more javadoc comments starting 
with „Add a row…“. Since you changed that to „Adds a row…“ twice, you might as 
well change it everywhere.

Hannes


Am 04.03.2020 um 23:15 schrieb Jonathan Gibbons <[email protected]>:

Please review a relatively simple change to the `add` methods for Content and 
its subtypes, to support chained usage.

The primary change is in Content (the last file listed in the webrev), where 
the return type of the `add` methods is changed from `void` to`Content. In 
addition, I added default methods to throw UnsupportedOperationException, and 
improved the doc comments.

Most of the other changes are consequences of the changes to Content. In 
particular, overriding methods are removed when the new default method provides 
the same behavior, and the return type of the remaining overriding methods is 
updated to be the return type of the overriding class ... i.e. using a 
covariant return.

The 3 small changes in doc comments in Table are unrelated. One is a 
grammatical fix; the others fix bad references to methods.

All the code is functionally equivalent to before; there are no changes 
required to any tests.

This changeset does not attempt to go and retrofit chained calls at possible 
use-sites.

-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8240137
Webrev: http://cr.openjdk.java.net/~jjg/8240137/webrev.00/index.html


Reply via email to