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

[Jim Graham]

On 5/20/16 3:33 PM, Kevin Rushforth wrote:
> 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.

Right, but (taking Arc as an example) Arc makes a specific reference to ArcHelper which turns around and hands a 
specific instance to its own instance field to a method that stores the value in the shapeHelper field.  How is that any 
different from just putting shapeHelper = ArcHelper.instance without 2 method calls and an accessor in the way?

Also, what if someone creates a custom sub-class of Shape?  (Not sure if that is supported or possible, but it is a 
public class with a public constructor so I don't think it is impossible.)

> 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.

It would be good to have a tool and/or automated test that warns about this.  Another reason is that the implicit 
constructor has no javadocs associated with it...

			...jim