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
