JDK-8157350 : Encapsulate impl_ methods in Shapes related classes
Type:Enhancement
Component:javafx
Sub-Component:graphics
Affected Version:9
Priority:P3
Status:Resolved
Resolution:Fixed
Submitted:2016-05-19
Updated:2016-05-24
Resolved:2016-05-21
The Version table provides details related to the release that this issue/RFE will be addressed.
Unresolved : Release in which this issue/RFE will be addressed. Resolved: Release in which this issue/RFE has been resolved. Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.
+1 on version .00 with the change to ArcHelper.java that Jim noticed (and, if you want, you can add my recommended additional comment in ShapeHelper).
20-05-2016
We looked through all the others and Arc was the only one.
20-05-2016
It sounds like you have a reason for the current helper paradigm as we move forward so that is good.
The naming anomalies aren't functional, but annoying.
So, no real open issues from my perspective. Note that Arc was the only helper I looked at so I can't say that there aren't other naming typos in there...
20-05-2016
I am OK with version .00 after the changes pointed out above. Let's make sure that the openjfx-dev thread with Jim's comments are addressed before proceeding.
20-05-2016
One thing I missed in my review that Jim pointed out is that there is a typo in ArcHelper.java:
49 public static void initHelper(Arc quadCurve) {
50 setHelper(quadCurve, getInstance());
51 }
that should be "arc" rather than "quadCurve".
20-05-2016
Looks good. Two minor comments (no need for a new webrev if you decide to fix them).
ShapeAccessor.java:
* You can also add the "TODO" comment here, since these will move to NodeAccessor:
115 ShapeHelper getHelper(Shape shape);
116 void setHelper(Shape shape, ShapeHelper shapeHelper);
* You have an extra blank line or two at the end of most of the new Helper classes (e.g., ArcHelper.java, ArcToHelper.java, CircleHelper.java)
[edited to remove the +1 while discussion is ongoing]
20-05-2016
Jim raised some points on the openjfx-dev list, so hold off pushing for now.
20-05-2016
Please review the proposed fix. In this fix I also did a minor fix to getMXWindowType() in Window and Stage including the helper classes to match the pattern we will use for overriding helper method.
webrev: http://cr.openjdk.java.net/~ckyang/JDK-8157350/webrev.00/