[9] Code Review Request For 8155762: Encapsulate JavaFX impl_* implementation methods in transform package

[Jim Graham]

TransformUtils.java - can't this class be deleted now since all it 
exists to do is to forward to TransformHelper?

Alternatively, why create TransformHelper in the first place when it is 
90% just a copy of what TransformUtils used to be (ignoring the inner 
class that got moved to Transform) with an accessor interface thrown in 
for good measure?  Just to change the name?  That could be done by using 
a source rename rather than gutting the old file and creating the new 
one from scratch...?  And we don't need both of them, do we?

Transform.java, line 2172 & 2187 - is there a reason to forward to the 
factory rather than just instantiating directly?

Transform.java, line 2177 & 2189 - I suppose the reason that the 
arguments are not declared to be Immutable in the first place is that 
some of these are used from other packages that have no visibility to 
the Immutable.  I was going to say that we should protect against them 
not being Immutables, but I guess these uses are hidden so it doesn't 
really affect anything by doing the cast - but it seems sloppy when 
looking at the code without looking everywhere to see how they are used 
(which I suppose is short-hand for "someone in the future might mess up 
because it's not obvious that these can't deal with any other type").

TransformHelper.java, line 108 & 112 - [formatting] perhaps move the 
m[xyz],tx to the next line for consistency between the two (like 68 & 76)?

The implementation of ImmutableTransform copies a fair bit of internals 
from Affine, but it could just wrap an Affine and save itself all of 
that work.  Arguably, we could make it a super-class of Affine and then 
it would own all of that code and Affine would subclass only for the 
modification methods, but we'd either have to expose it as a public 
class or have an unexplained inaccessible superclass on Affine...? 
(Also, it's not clear if public methods on a non-public superclass of 
Affine would be visible through Affine so Affine would either have to 
have forwarding methods or we'd have to make the super class public).

On the other hand, what would be wrong with having a public BaseAffine 
that was Affine without its modification methods, and having Affine 
subclass it?  It would be effectively Immutable, but using the word 
Immutable in its name might sound like a promise which would be violated 
by its subclass - better to leave that property implied by its lack of 
mutating methods and possibly mentioning the intent in the javadoc...

			...jim

On 5/3/2016 11:24 PM, Chien Yang wrote:
> Kevin and Jim,
>
> Please review the proposed fix:
>
> Webrev: https://bugs.openjdk.java.net/browse/JDK-8155762
> JIRA: http://cr.openjdk.java.net/~ckyang/JDK-8155762/webrev.00/
>
> Thanks,
> - Chien