Re: [OpenJDK 2D-Dev] RFR: 8250855: Address reliance on default constructors in the Java 2D APIs
It is an anti-pattern to rely on an implicit default constructor in a publicly exported class in a library. There are (at least) three good reasons to avoid this: 1. The default constructor will have no API docs 2. You could end up with a public constructor in a class where you don't want a publicly visible constructor, for example a class with factory methods. 3. In a class where you do intend a public constructor, if you later add another constructor with an argument, it will have the effect of removing the default one, which is an incompatible API change. -- Kevin On 9/16/2020 11:53 PM, Peter Hull wrote: Just for my curiosity, what issues can arise relying on default constructors? I couldn't find anything with google (apart from links back to these messages!) Thanks, Peter
Re: [OpenJDK 2D-Dev] RFR: 8250855: Address reliance on default constructors in the Java 2D APIs
Just for my curiosity, what issues can arise relying on default constructors? I couldn't find anything with google (apart from links back to these messages!) Thanks, Peter
Re: [OpenJDK 2D-Dev] RFR: 8250855: Address reliance on default constructors in the Java 2D APIs
On Tue, 15 Sep 2020 09:38:02 GMT, Conor Cleary wrote: >> Marked as reviewed by serb (Reviewer). > > Now awaiting CSR approval as advised The CSR needs some updates to put the spec inline - PR: https://git.openjdk.java.net/jdk/pull/153
Re: [OpenJDK 2D-Dev] RFR: 8250855: Address reliance on default constructors in the Java 2D APIs
On Mon, 14 Sep 2020 21:27:46 GMT, Sergey Bylokhov wrote: >> This issue relates to JDK-8250639 '☂ Address reliance on default >> constructors in the java.desktop module'. The changes >> address the reliance on default constructors by adding in basic constructors >> in the following classes: >> - java.awt.Image >> - java.awt.PrintJob >> - java.awt.font.GlyphVector >> - java.awt.font.LayoutPath >> - java.awt.font.LineMetrics >> - java.awt.image.AbstractMultiResolutionImage >> - java.awt.image.BufferStrategy >> - java.awt.image.ImageFilter >> - java.awt.image.RGBImageFilter >> - java.awt.image.VolatileImage >> - javax.print.PrintServiceLookup >> - javax.print.ServiceUI >> - javax.print.ServiceUIFactory >> - javax.print.StreamPrintServiceFactory >> - javax.print.event.PrintJobAdapter >> >> specdiff: >> http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.02/specdiff/overview-summary.html >> csr: >> https://bugs.openjdk.java.net/browse/JDK-8252495 > > Marked as reviewed by serb (Reviewer). Now awaiting CSR approval as advised - PR: https://git.openjdk.java.net/jdk/pull/153
Re: [OpenJDK 2D-Dev] RFR: 8250855: Address reliance on default constructors in the Java 2D APIs
On Mon, 14 Sep 2020 14:32:18 GMT, Conor Cleary wrote: > This issue relates to JDK-8250639 '☂ Address reliance on default constructors > in the java.desktop module'. The changes > address the reliance on default constructors by adding in basic constructors > in the following classes: > - java.awt.Image > - java.awt.PrintJob > - java.awt.font.GlyphVector > - java.awt.font.LayoutPath > - java.awt.font.LineMetrics > - java.awt.image.AbstractMultiResolutionImage > - java.awt.image.BufferStrategy > - java.awt.image.ImageFilter > - java.awt.image.RGBImageFilter > - java.awt.image.VolatileImage > - javax.print.PrintServiceLookup > - javax.print.ServiceUI > - javax.print.ServiceUIFactory > - javax.print.StreamPrintServiceFactory > - javax.print.event.PrintJobAdapter > > specdiff: > http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.02/specdiff/overview-summary.html > csr: > https://bugs.openjdk.java.net/browse/JDK-8252495 Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/153
Re: [OpenJDK 2D-Dev] RFR: 8250855: Address reliance on default constructors in the Java 2D APIs
On Mon, 14 Sep 2020 19:02:27 GMT, Phil Race wrote: >> This issue relates to JDK-8250639 '☂ Address reliance on default >> constructors in the java.desktop module'. The changes >> address the reliance on default constructors by adding in basic constructors >> in the following classes: >> - java.awt.Image >> - java.awt.PrintJob >> - java.awt.font.GlyphVector >> - java.awt.font.LayoutPath >> - java.awt.font.LineMetrics >> - java.awt.image.AbstractMultiResolutionImage >> - java.awt.image.BufferStrategy >> - java.awt.image.ImageFilter >> - java.awt.image.RGBImageFilter >> - java.awt.image.VolatileImage >> - javax.print.PrintServiceLookup >> - javax.print.ServiceUI >> - javax.print.ServiceUIFactory >> - javax.print.StreamPrintServiceFactory >> - javax.print.event.PrintJobAdapter >> >> specdiff: >> http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.02/specdiff/overview-summary.html >> csr: >> https://bugs.openjdk.java.net/browse/JDK-8252495 > > Marked as reviewed by prr (Reviewer). This needs the CSR to be approved before integration - PR: https://git.openjdk.java.net/jdk/pull/153
Re: [OpenJDK 2D-Dev] RFR: 8250855: Address reliance on default constructors in the Java 2D APIs
On Mon, 14 Sep 2020 14:32:18 GMT, Conor Cleary wrote: > This issue relates to JDK-8250639 '☂ Address reliance on default constructors > in the java.desktop module'. The changes > address the reliance on default constructors by adding in basic constructors > in the following classes: > - java.awt.Image > - java.awt.PrintJob > - java.awt.font.GlyphVector > - java.awt.font.LayoutPath > - java.awt.font.LineMetrics > - java.awt.image.AbstractMultiResolutionImage > - java.awt.image.BufferStrategy > - java.awt.image.ImageFilter > - java.awt.image.RGBImageFilter > - java.awt.image.VolatileImage > - javax.print.PrintServiceLookup > - javax.print.ServiceUI > - javax.print.ServiceUIFactory > - javax.print.StreamPrintServiceFactory > - javax.print.event.PrintJobAdapter > > specdiff: > http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.02/specdiff/overview-summary.html > csr: > https://bugs.openjdk.java.net/browse/JDK-8252495 Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/153
Re: [OpenJDK 2D-Dev] RFR[8250855]: 'Address reliance on default constructors in the Java 2D APIs'
Hi, Conor The change looks fine, please note that you will need to crate a PR on the GitHub to integrate this fix. * CSR: https://bugs.openjdk.java.net/browse/JDK-8252495The CSR could be improved, take a look to this example: https://bugs.openjdk.java.net/browse/JDK-8250581 The body of the CSR contains all changes in the specification, the link to the webrev is not enough. -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] RFR[8250855]: 'Address reliance on default constructors in the Java 2D APIs'
Hi everyone.
Thanks for the feedback!
Firstly, I changed the wording from 'Creates' to 'Constructs' as per
Philip's suggestion (and corrected a spelling mistake).
Secondly, for the protected constructors (in the abstract classes) I
used the wording "Constructor for subclasses to call." as that seems to
be the norm now.
* webrev:
http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.02/
* specdiff:
http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.02/specdiff/overview-summary.html
* CSR: https://bugs.openjdk.java.net/browse/JDK-8252495
* bug: https://bugs.openjdk.java.net/browse/JDK-8250859
Regards,
Conor
On 31/08/2020 21:15, Philip Race wrote:
Right we have started to be consistent using "Constructor for
subclasses to call":
Also I prefer constructs over creates, even for the concrete classes,
eg this :
+
+ /**
+ * Creates an {@code ImageFilter}.
+ */
+ public ImageFilter() {}
+
should be "Constructs an {@code ImageFilter}"
-phil.
On 8/28/20, 5:29 PM, Sergey Bylokhov wrote:
Hi, Conor.
Please use such spec for the protected constructor: "Constructor for
subclasses to call":
https://cr.openjdk.java.net/~psadhukhan/8250850/webrev.1/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalTheme.java.sdiff.html
Actually the current text is also fine to me, but looks like many
people use the text above as a description.
On 28.08.2020 01:28, Conor Cleary wrote:
Hello all,
Could someone please review my changes for JDK-8250855, 'Address
reliance on default constructors in the Java 2D APIs'? This issue
relates to JDK-8250639 '☂ Address reliance on default constructors
in the java.desktop module'. The changes address the reliance on
default constructors by adding in basic constructors in the
following classes:
* java.awt.Image
* java.awt.PrintJob
* java.awt.font.GlyphVector
* java.awt.font.LayoutPath
* java.awt.font.LineMetrics
* java.awt.image.AbstractMultiResolutionImage
* java.awt.image.BufferStrategy
* java.awt.image.ImageFilter
* java.awt.image.RGBImageFilter
* java.awt.image.VolatileImage
* javax.print.PrintServiceLookup
* javax.print.ServiceUI
* javax.print.ServiceUIFactory
* javax.print.StreamPrintServiceFactory
* javax.print.event.PrintJobAdapter
A key issue is the accompanying description for each of the added
constructors and is probably the feedback I would value most as it
has been a point of discussion previously. I have included a
specdiff to easily view the changes observed in the api
documentation. Currently drafting a CSR for these changes.
* webrev:
http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.01/
* specdiff:
http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.01/specdiff/overview-summary.html
* bug: https://bugs.openjdk.java.net/browse/JDK-8250855
Best Regards,
-Conor
Re: [OpenJDK 2D-Dev] RFR[8250855]: 'Address reliance on default constructors in the Java 2D APIs'
Right we have started to be consistent using "Constructor for subclasses
to call":
Also I prefer constructs over creates, even for the concrete classes, eg
this :
+
+/**
+ * Creates an {@code ImageFilter}.
+ */
+public ImageFilter() {}
+
should be "Constructs an {@code ImageFilter}"
-phil.
On 8/28/20, 5:29 PM, Sergey Bylokhov wrote:
Hi, Conor.
Please use such spec for the protected constructor: "Constructor for
subclasses to call":
https://cr.openjdk.java.net/~psadhukhan/8250850/webrev.1/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalTheme.java.sdiff.html
Actually the current text is also fine to me, but looks like many
people use the text above as a description.
On 28.08.2020 01:28, Conor Cleary wrote:
Hello all,
Could someone please review my changes for JDK-8250855, 'Address
reliance on default constructors in the Java 2D APIs'? This issue
relates to JDK-8250639 '☂ Address reliance on default constructors in
the java.desktop module'. The changes address the reliance on default
constructors by adding in basic constructors in the following classes:
* java.awt.Image
* java.awt.PrintJob
* java.awt.font.GlyphVector
* java.awt.font.LayoutPath
* java.awt.font.LineMetrics
* java.awt.image.AbstractMultiResolutionImage
* java.awt.image.BufferStrategy
* java.awt.image.ImageFilter
* java.awt.image.RGBImageFilter
* java.awt.image.VolatileImage
* javax.print.PrintServiceLookup
* javax.print.ServiceUI
* javax.print.ServiceUIFactory
* javax.print.StreamPrintServiceFactory
* javax.print.event.PrintJobAdapter
A key issue is the accompanying description for each of the added
constructors and is probably the feedback I would value most as it
has been a point of discussion previously. I have included a specdiff
to easily view the changes observed in the api documentation.
Currently drafting a CSR for these changes.
* webrev:
http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.01/
* specdiff:
http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.01/specdiff/overview-summary.html
* bug: https://bugs.openjdk.java.net/browse/JDK-8250855
Best Regards,
-Conor
Re: [OpenJDK 2D-Dev] RFR[8250855]: 'Address reliance on default constructors in the Java 2D APIs'
Hi, Conor. Please use such spec for the protected constructor: "Constructor for subclasses to call": https://cr.openjdk.java.net/~psadhukhan/8250850/webrev.1/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalTheme.java.sdiff.html Actually the current text is also fine to me, but looks like many people use the text above as a description. On 28.08.2020 01:28, Conor Cleary wrote: Hello all, Could someone please review my changes for JDK-8250855, 'Address reliance on default constructors in the Java 2D APIs'? This issue relates to JDK-8250639 '☂ Address reliance on default constructors in the java.desktop module'. The changes address the reliance on default constructors by adding in basic constructors in the following classes: * java.awt.Image * java.awt.PrintJob * java.awt.font.GlyphVector * java.awt.font.LayoutPath * java.awt.font.LineMetrics * java.awt.image.AbstractMultiResolutionImage * java.awt.image.BufferStrategy * java.awt.image.ImageFilter * java.awt.image.RGBImageFilter * java.awt.image.VolatileImage * javax.print.PrintServiceLookup * javax.print.ServiceUI * javax.print.ServiceUIFactory * javax.print.StreamPrintServiceFactory * javax.print.event.PrintJobAdapter A key issue is the accompanying description for each of the added constructors and is probably the feedback I would value most as it has been a point of discussion previously. I have included a specdiff to easily view the changes observed in the api documentation. Currently drafting a CSR for these changes. * webrev: http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.01/ * specdiff: http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8250855/webrevs/webrev.01/specdiff/overview-summary.html * bug: https://bugs.openjdk.java.net/browse/JDK-8250855 Best Regards, -Conor -- Best regards, Sergey.
