Hello Jan
I had a quick look at your work. Sorry for being so late :(
WarpGridTransform2D
-------------------
Maybe we don't need to keep around the xStart, xStep, etc. private final fields,
since we can get them through warp.getXStart(), warp.getXStep(), etc. methods.
Unless there is a performance reason, but those fields seem to be used in
getParameterValues() (not performance-critical), or methods like
calculateInverse (should probably be invoked only once). The later and its
friend can can the values they are interrested in once for ever as a local final
variable:
final int xStart = warp.getXStart().
WarpGridTransform2D.transform
-----------------------------
The method should declares "throws TransformException", and all "catch
(TransformException e) {e.printStackTrace()}" block should be removed.
I also noticed that in current implementation, the first call to
"worldToGrid.transform" overwrite the user source coordinates, which is probably
not what the user wants. See next point.
WarpGridTransform2D.worldToGrid
-------------------------------
Should we really carry this information? Current WarpGridTransform2D.transform
implementation duplicates the work of ConcatenatedTransform in a less elaborated
way. I would suggest to remove "worldToGrid" from WarpGridTransform2D and remove
the transform method overriding as well. Instead, the factory could creates
automatically a chain of WarpGridTransform2D and ConcatenatedTransform as
needed.
WarpGridTransform2D.calculateInverse
------------------------------------
Maybe is should be private (and maybe static?) for now.
WarpGridTransform2D.Provider
----------------------------
According Java naming convention, static final fields like xStart should be
upper case (X_START). Then, line like below in createMathTransform:
final int XSTART = intValue(xStart, values);
would be
final int xStart = intValue(X_START, values);
WarpGridTransform2D.ProviderFile
--------------------------------
Be sure to avoid everything thats look like:
try {
doSomething
} catch (SomeException e) {
e.printStackTrace();
}
We needs to declare the exception in the method, or make some action more
elaborated than just printing the stack trace (e.g. fallback on some default
values). But we can not just let the program continue as if no exception
occured.
WarpGridBuilder
---------------
A little bit more javadoc would be nice. For example method "getGrid()" returns
"float[]", but the javadoc just said "Returs Grid". How is the grid computed?
(by calling "computeWarpGrid" from what I can see from the code). How data are
organized in the returned float[] array?
Cheers,
Martin
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
