On Thu, 24 Nov 2022 11:57:33 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> 
wrote:

>> 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.

(to be clear, some of the above comments refer to the code that was already 
there before your changes)

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

PR: https://git.openjdk.org/jdk/pull/11260

Reply via email to