Hello Jan

What about the following modification?

1) Replace CRSException by the following one (which in my understanding reflect 
the kind of error involved): 
http://geoapi.sourceforge.net/pending/javadoc/org/opengis/spatialschema/geometry/MismatchedReferenceSystemException.html

2) Rename "AbstractParamCalculator" as "TransformParameterCalculator" or just 
"ParameterCalculator" (or what about "MathTransformBuilder"?). I believe that 
the "Abstract" prefix is not required here since we are not implementing an 
interface. For example java.io.InputStream, java.awt.Component, etc. doesn't 
have an "Abstract" prefix.

3) In "<Whatever>ParameterCalculator", add two public abstract methods: 
'getMinimumPointCount()' (or something else if you have a better name in mind), 
and 'getPointsDimension()'. Those methods should be implemented in subclass and 
returns (hard-coded) the value currently provided in 'checkPoints' method 
calls. 
The 'checkPoints()' method could now become a no-argument method, which can be 
invoked straight from 'setSourcePoints' and 'setDestinationPoints'. The purpose 
is to provide the user with some useful informations (the minimum number of 
points he need to provide), and allow earlier error detection (at 
'setSourcePoint(...)' or 'setDestinationPoint(...)' call, rather than at 
'getMathTransform()' call). Note: 'setSourcePoints(...)' and 
'setDestinationPoints(...)' can check the points dimensions and CRS, but the 
check for ptSrc.length == ptDst.length will probably still need to be delay to 
'getMathTransform()' (or some other computational method), since we need to 
have 
both arrays set before we can perform this check.

4) In "<Whatever>ParameterCalculator", turn the 'protected CRS' field into a 
private one, and add a public 'getCoordinateReferenceSystem()' method instead. 
The purpose is to prevent subclasses from assigning a value to this field. Only 
'checkPoints()' or some related method should modify this field.

On a related note, I don't believe that srcPts and dstPts should have the same 
CRS. A math transform is usually for transforming points from one CRS to an 
other CRS. If the source and destination points had the same CRS, then the math 
transform should be the identity transform! Consequently, I think that the 
current 'CRS' field should actually be splitted in two fields: 'sourceCRS' and 
'targetCRS'.

Finally, I commited a package.html file in 
org.geotools.referencing.operation.calculator. Would it be possible to 
copy-and-paste some explanation from yours FOSS4G poster if possible?

About the changes enumerated in this email, if you agree with them but don't 
have the time, I can apply them since they are mostly cosmetic changes quite 
fast to apply, and it give me an opportunity to explore your code. At your 
choice :)

        Martin

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to