Sounds good...
...jim
On 5/20/16 3:56 PM, Kevin Rushforth wrote:
Jim Graham wrote:
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?
But the shapeHelper field is in the base Shape class not in the Arc class. If
we wanted a different pattern for classes
in the same package as the base class from classes in a different package then
I guess I can see how this is solvable by
making the ArcHelper.getInstance() method public and having the Arc()
constructors call the package-scope
setHelper(ShapeHelper) method in Shape, but as soon as Chien move the stored
helper instance up to Node (which is the
next step) it would stop working.
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.)
Since we don't have public API for many of the things they would need even
today, an application isn't able to do that.
They couldn't really do it anyway before this change, since impl_getPeer() and
several other methods aren't
implementable by an application (NGNode is not publicly exported for example).
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...
Indeed. I wonder if doclint will help (we are in the process of making our docs
doclint clean).
-- Kevin
...jim
