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

[Kevin Rushforth]

This is needed for those cases where we need to encapsulate a method in 
the base Shape class that used to be public and overridden in the 
subclasses, not all of which are in the same package. It may seem like 
overkill, but we need a way to associate the the Shape instance of a 
particular subtype with the helper instance of the correct subtype. Each 
class in the hierarchy calls the specific XxxxxHelper.initHelper(this) 
method so that it can store back an instance of the right helper in the 
base class. A package-private method wouldn't work given that some 
shapes (e.g., Text) are in different packages.

As for "Arc quadCurve" that looks like cut and paste errors; I missed 
that in my review, so will go back and look at the rest.

Good reminder about the implicit "public Shape()" constructor. Chien 
already had to add an explicit public no-arg constructor in two classes. 
We really shouldn't rely on the implicit constructor in our public 
classes, since it makes it easy to make such a mistake.

-- Kevin



Jim Graham wrote:
> 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