[9] Code Review Request For 8157350: Encapsulate impl_ methods in Shapes related classes

Jim Graham james.graham at oracle.com
Fri May 20 21:43:15 UTC 2016


It feels like there is extra indirection for the code that sets the helpers.  Is there a reason it isn't as simple as 
"this.shapeHelper = FooHelper.instance"?  Or, even a package-private Shape(ShapeHelper) constructor on Shape?  (*)

Also, many of the helper classes have argument names that were obviously cut and pasted from a different shape class. 
The names don't functionally matter, but seeing the ArcHelper manipulate variables called "quadThisOrThat" is a little 
jarring...

			...jim

(*) Note that since Shape currently has a public empty constructor that is added implicitly by the fact that it includes 
no explicit constructors you have to make sure to make an explicit one if you ever add an explicit constructor...

On 5/20/16 2:11 PM, Chien Yang wrote:
> Hi Kevin,
>
> Please review the proposed fix as we have discussed.
>
> JIRA: https://bugs.openjdk.java.net/browse/JDK-8157350
> Webrev: http://cr.openjdk.java.net/~ckyang/JDK-8157350/webrev.00/
>
> Thanks,
> - Chien


More information about the openjfx-dev mailing list