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.
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.
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.
The benefit would be that we wouldn't have all the variables in a subclass that doesn't use them (it's cleaner). Finally, the API could optionally also be changed to use interfaces instead of abstract classes.
Regards
Niels