I'm very late, but I finally started to review the coverage branch merge. I
would like to hold on the 2.3 release until I finished this review, since I
have
some questions.
org.geotools.coverage.CategoryList
----------------------------------
The implementation before merge was looking for a "no data" category using
Double.isNaN(geophysicsValue) as a criterion ("geophysics values" not to be
confused with "pixel values", which can't be NaN). The coverage branch merge
added a check based on the "no data" name in current local, regardless of
Double.isNaN(...). Should we move this check into the "if (Double.isNaN(...))"
block? By definition, a "no data" category maps to NaN geophysics values. If it
maps to real values, then it is not a "no data" category anymore.
Is there any use case for selecting a "no data" category which maps to real
geophysics values? Or can I move the check into the Double.isNaN(...) block?
org.geotools.coverage.grid.GeneralGridGeometry
----------------------------------------------
The coverage branch merge turned a number of "package private static" methods
into public ones:
- reverse
- swapXY
Do those methods really need to be public? They were for internal purpose, and
it is not clear to me what a user way want to do with 'reverse' for example.
Additionnaly, I noticed that the coverage branch merge added new public static
method, with code that were previously done by constructors:
- getEnvelope(GridRange, MathTransform, CoordinateReferenceSystem, boolean)
- getTransform(GridRange, Envelope, boolean[], boolean, boolean)
The signature are identical to the constructor signature except for the
additional boolean argument. Actually, those public static method are just
shorthand for the following existing methods:
- new GeneralGridGeometry(gridRange, mathTransform, crs).getEnvelope();
- new GeneralGridGeometry(gridRange, envelope, reverse,
swapXY).getTransform();
Their only advantage is to avoid the temporary creation of a
GeneralGridGeometry
object, at the cost of more complex API (at least duplication in methods
signature) and less flexibility (are we going to add a 'getEnvelope(...)' or
'getTransform(...)' method every time we add a new constructor signature?)
Unless there is a real performance bootleneck, can we ask peoples to use the
new
GridGeometry(...).getFoo() idiom, which is more extensible? The only thing we
would need to add is a
getTransform(PixelOrientation)
method in order to provides the functionality provided by the new 'boolean
halfPix' argument, where PixelOrientation is:
http://geoapi.sourceforge.net/snapshot/javadoc/org/opengis/metadata/spatial/PixelOrientation.html
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
