______________________________________________________________ > Od: [EMAIL PROTECTED] > Komu: Jan Jezek <[EMAIL PROTECTED]> > CC: [email protected] > Datum: 14.10.2006 03:10 > Předmět: Minor changes proposal for the calculator package > >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/MismatchedRefe>renceSystemException.html >
I agree. >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. > I agree, MathTransformBuilder sounds good. >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. I agree. > >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. > I agree. >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'. Yes. My mistake. > >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? > Ok. I will try to make it and I'll upload it. >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 :) I would be happy if you will make these changes. There are also tests on caching the Exceptions so they also have to be changed. Thanks a lot for such complex code review! Cheers, Jan. ------------------------------------------------------------------------- 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
