On Tue, 29 Nov 2022 16:46:43 GMT, Per Minborg <pminb...@openjdk.org> wrote:

> This PR proposes a variety of modernisations to the `jdk.sctp` module.
> 
> During the fix of https://bugs.openjdk.org/browse/JDK-8296024, several 
> improvement areas were identified including: 
> 
> * Replacing duplicate code segments 
> * Making certain fields final 
> * Using enhanced switch 
> * Using records 
> * Fixing typos 
> * Marking fields participating in serialisation with `@Serial` 
> * Modernizing toString() implementations 
> * Using pattern matching 
> * Using diamond operators

src/jdk.sctp/aix/classes/sun/nio/ch/sctp/SctpChannelImpl.java line 39:

> 37: import com.sun.nio.sctp.SctpSocketOption;
> 38: 
> 39: import static sun.nio.ch.sctp.Util.sctpNotSupported;

Not really a fan of static imports like this; it looks like a local method in 
the code but is not. 
In this case, the code would be more readable as `Util.sctpNotSupported()`.
This style of creating exceptions to throw is unusual in OpenJDK.

src/jdk.sctp/aix/classes/sun/nio/ch/sctp/Util.java line 1:

> 1: package sun.nio.ch.sctp;

Can this be shared in **share**/classes/sun/nio/sctp/Util.java?

src/jdk.sctp/macosx/classes/sun/nio/ch/sctp/SctpChannelImpl.java line 44:

> 42:  * Unimplemented.
> 43:  */
> 44: public class SctpChannelImpl extends SctpChannel {

Going a bit beyond just updating syntax; but that's a different PR...
There could be more sharing of the unsupported implementation if SctpChannel 
was not abstract and its method bodies threw the exception.  There would less 
duplication for unsupported platforms.

src/jdk.sctp/unix/classes/sun/nio/ch/sctp/SctpMultiChannelImpl.java line 722:

> 720: 
> 721:     /**
> 722:      * @throws IllegalArgumentException If the given association is not 
> controlled by this channel

There should be a 1st sentence describing the function; its odd to see only an 
@throws.

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

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

Reply via email to