On Tue, Jan 14, 2014 at 2:33 PM, Niels Charlier <[email protected]> wrote:
> Hello Mailing List,
>
> I am writing a geopkg coverage reader. A geopkg can have many coverages,
> each of which can have their own envelope, CRS, resolutions, ...
>
> Currently the API works as follows.
> - The GridFormatFactorySpi Service returns a AbstractGridFormat (not the
> GridFormat interface but an abstract class)
> - The AbstractGridFormat class returns an AbstractGridCoverage2DReader
> (not the GridCoverage2DReader interface but an abstract class).
> AbstractGridCoverage2DReader contains lots of implementation, so we have
> a bit of a mixture between API and implementation here.
>
> The AbstractGridCoverage2DReader was written we the assumption of a
> single coverage per data source.
> Recently, the GridCoverage2DReader was changed to allow multiple
> coverages, now being used by netcdf. However netcdf still must derive
> from the AbstractGridCoverage2DReader for the reasons above, however
> overrides the necessary methods to allow multiple coverages. This seems
> to work fine for netcdf, but there are issues for geopkg.
> AbstractGridCoverage2DReader uses in many methods variables called
> 'crs', 'originalEnvelope', 'highestResolution', ... all under the
> assumption there is only one of those.
>
Well.. no, the methods in question can be overridden, this means you
can have them work with multiple coverages:
@Override
public GeneralEnvelope getOriginalEnvelope(String coverageName) {
if(!checkName(coverageName)){
throw new IllegalArgumentException("The specified coverageName
" + coverageName + "is not supported");
}
return getOriginalEnvelope();
}
/**
* Retrieves the {@link GeneralEnvelope} for this
* {@link AbstractGridCoverage2DReader}.
*
* @return the {@link GeneralEnvelope} for this
* {@link AbstractGridCoverage2DReader}.
*/
public GeneralEnvelope getOriginalEnvelope() {
return new GeneralEnvelope(originalEnvelope);
}
What's stopping you from overriding getOriginalEnvelope(String
coverageName)?
The reason why getOriginalEnvelope() (without the coverage name) was
kept was to avoid breaking all of the existing client code just for the sake
of NetCDF.
If your implemetation does not have a shared original envelope you just
throw an unsupported operation exception for the moment, and find out
what works and what does not on top of it (there might still be code around
that does not know about coverage names).
>
> My proposal to solve this problem - without a change in the API - is to
> replace all occurrences of these variables in
> AbstractGridCoverage2DReader to a call to functions. For example an
> occurence of 'crs' in the code would be replaced by a call to
> 'getCoordinateReferenceSystem(coverageName)'. All methods dependant on
> these functions would take a 'coverageName' as argument, but a second
> function without that argument would remain for backward compatibility,
> using the 'coverageName' variable as argument. All the variables would
> remain, the default implementation would be to return those variables.
> This way geopkg can simply override the functions and ignore the
> variables. But there would be perfect backward compatibility towards the
> subclasses that use the variables.
>
But... isn't this what we have now?
@Override
public CoordinateReferenceSystem getCoordinateReferenceSystem(String
coverageName) {
if(!checkName(coverageName)){
throw new IllegalArgumentException("The specified coverageName
" + coverageName + "is not supported");
}
// Default implementation for backwards compatibility
return getCoordinateReferenceSystem();
}
@Override
public CoordinateReferenceSystem getCoordinateReferenceSystem() {
return getCrs();
}
>
> This would work by itself, but could also be a first step towards
> refactoring the API. Additionally, but not necessarily, we could split
> the AbstractGridCoverage2DReader into two classes;
> - AbstractGridCoverage2DReader would have an abstract
> getCoordinateReferenceSystem(coverageName) method. The geopkg reader
> would derive directly from this.
> - A subclass, for example SingleGridCoverage2DReader would make the
> default implementations using the variables under the assumption that
> there is only one coverage. All the existing single-coverage readers
> would then derive from SingleGridCoverage2DReader.
>
This approach seems sensible.
Cheers
Andrea
--
== Our support, Your Success! Visit http://opensdi.geo-solutions.it for
more information ==
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
-------------------------------------------------------
------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
