RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v6]

Markus KARG github.com+1701815+mkarg at openjdk.java.net
Thu Jul 29 08:11:33 UTC 2021


On Tue, 27 Jul 2021 00:57:02 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Markus KARG has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR.
>
> src/java.base/share/classes/sun/nio/ch/ChannelOutputStream.java line 85:
> 
>> 83:     private byte[] bs;       // Invoker's previous array
>> 84:     private byte[] b1;
>> 85: 
> 
> It might be better to put the field declarations at the beginning of the class before the static methods.

This is a question of style and personal likes. Code maintenance is much easier if variables are defined *near* to its first use, not *far away* (as in the Pascal or C language). If the reviewers want me to change it, I will do it.

> test/jdk/sun/nio/ch/ChannelInputStream/TransferTo.java line 67:
> 
>> 65:         test(readableByteChannelInput(), defaultOutput());
>> 66:     }
>> 67: 
> 
> This test looks like it's doing what it's supposed to do. I'm not asking to change it, but I think using TestNG might have given a simpler result with less work. For example, there could be one `DataProvider` supplying inputs which feed a `@Test` which expects an NPE, and another `DataProvider` supplying input-output pairs which feeds a `@Test` which validates the functionality.

No doubt, using modern tools would have spared me work, and indeed I would have chosen JUnit or TestNG if there would be a clear commitment to that tool *upfront*. For now, I see now use in reworking the code afterwards.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4263


More information about the core-libs-dev mailing list