On Fri, 28 Oct 2022 03:42:05 GMT, Prasanta Sadhukhan <[email protected]>
wrote:
>> The behavior of MatteBorder constructors taking Insets object as a parameter
>> is undocumented. It would throw NPE if null object is passed to it, which
>> should be documented in the spec.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Review fix
In addition to the comments in the code, could you please update the copyright
year and remove `Rectangle` from imports in `EmptyBorder`?
src/java.desktop/share/classes/javax/swing/border/AbstractBorder.java line 89:
> 87: * @return the <code>insets</code> object
> 88: * @throws {@code NullPointerException} if {@code insets}
> 89: * is {@code null}
This does not compile, `@throws` expects a “plain” class name, monospaced font
is applied automatically to `NullPointerException`.
src/java.desktop/share/classes/javax/swing/border/AbstractBorder.java line 91:
> 89: * is {@code null}
> 90: */
> 91: public Insets getBorderInsets(Component c, Insets insets) {
Should the specification be clarified here? The method
`getBorderInsets(Component c)` above says the fields of the returned `Insets`
object are set to 0. The implementation of `getBorderInsets(Component c, Insets
insets)` sets the fields of `Insets` to zero … because it has no other insets.
Should the specification state that this *default implementation* sets the
fields of the `Insets` object to zero?
src/java.desktop/share/classes/javax/swing/border/EmptyBorder.java line 108:
> 106: * @param insets the object to be reinitialized
> 107: * @throws {@code NullPointerException} if {@code insets}
> 108: * is {@code null}
Suggestion:
* @throws NullPointerException {@inheritDoc}
I suggest using `{@inheritDoc}` to copy the exception description from the
super class for both `EmptyBorder` and `MatteBorder`. (It cannot be used for
constructors though.)
-------------
Changes requested by aivanov (Reviewer).
PR: https://git.openjdk.org/jdk/pull/10740
