On Thu, 24 Nov 2022 11:54:43 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> Per Minborg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove redundant method
>
> src/java.base/share/classes/sun/nio/ch/IOUtil.java line 519:
>
>> 517: }
>> 518:
>> 519: record LinkedRunnable(Runnable node, Runnable next)
>
> Some (optional) style comments: I'm not sure this is a record. E.g. both this
> and the `Releaser` class are only used externally to call their `run` method.
> The fact that they are made up of components is immaterial to clients, which
> seems to suggest that an interface would be better - at least subjectively.
> If I were to write the code I will declare the implementation inside the
> factories, as local/anon classes.
Separately, also (optional) stylistic: the indenting on this and following
class is a bit odd. There is a certain tendency to compress lines - e.g. to
reach for one-liners, when in reality the body is not that trivial. If I were
to write the code I'd insert a newline after the `{` and I'd also break apart
initialization (e.g. no two statements separated by `;` in the same line).
Also, I noted that you start the body of the record (e.g. `{`) on a new line,
which I also find odd.
-------------
PR: https://git.openjdk.org/jdk/pull/11260
