Clean up Generics from DataStore - STEMLab/geotools GitHub Wiki
-
Motivation: Try out Csv2Shp tutorial with someone new, now try it with someone new to Java
-
Contact: full name
-
Tagline: I have had enough of generics being used in the core experience of new users; it makes the code complicated.
The DataAccess super class for DataStore had a couple of options for introducing geoapi feature into the codebase; and chose to do so with the minimal number of clasess. To accomplish this the use of generics was used to allow those classes to be specific.
Current relationship:
public interface DataAccess<T,F> {
...
FeatureSource<T,F> getFeatureSource(Name)
T getSchema(Name);
}
public interface DataStore extends DataAccess<SimpleFeatureType, SimpleFeature> {
...
}
Win? Well DataStore is "clean" and did not require more detail, but ... the basic example for geotools now looks like this:
String typeName = newDataStore.getTypeNames()[0];
FeatureSource<SimpleFeatureType,SimpleFeature> featureSource =
(<SimpleFeatureType,SimpleFeature>) newDataStore.getFeatureSource(typeName);
if (featureSource instanceof FeatureStore<?,?>) {
FeatureStore<SimpleFeatureType,SimpleFeature> featureStore =
(FeatureStore<SimpleFeatureType,SimpleFeature>) featureSource;
...
}
} else {
System.out.println(typeName + " does not support read/write access");
System.exit(1);
}
It get's worse when you add FeatureCollection<SimpleFeatureType,SimpleFeature> into the mix...
The other option for DataAccess super class for DataStore was to introduce super classes for FeatureSource and use type narrowing rather then generics:
public interface DataAccess {
...
Source getFeatureSource(Name)
FeatureType getSchema(Name);
}
public interface DataStore extends DataAccess<SimpleFeatureType, SimpleFeature> {
FeatureSource getFeatureSource(Name);
SimpleFeatureType getSchema();
}
That is water under the bridge (we cannot change now without breaking client code) - but the approach still needs to be used.
public interface DataAccess<T,F> {
...
FeatureSource<T,F> getFeatureSource(Name)
T getSchema(Name);
}
public interface DataStore extends DataAccess<SimpleFeatureType, SimpleFeature> {
SimpleFeatureSource getFeatureSource(Name)
SimpleFeatureType getSchema( Name );
}
public interface SimpleFeatureSource extends FeatureSource<SimpleFeatureType,SimpleFeature>{
// no changed api
}
public interface SimpleFeatureStore extends FeatureStore<SimpleFeatureType,SimpleFeature>, SimpleFeatureSource {
// no changed api
}
public interface SimpleFeatureSource extends SimpleFeatureSource<SimpleFeatureType,SimpleFeature>, SimpleFeatureStore {
// no changed api
}
This does not represent any new methods; SimpleFeatureSource simply extends FeatureSource<SimpleFeatureType,SimpleFeature>; it represents the same API and does not break any client code. Some of the implementation code does have to change; but if they are extending geotools implementations the above patch takes care of it.
There is small patch for just the change up to this point: https://osgeo-org.atlassian.net/browse/GEOT-3051 patch simplefeaturesource.patch
The other half of the mess is all the generics around the use of FeatureCollection.
The approach is the same:
- introduce SimpleFeatureCollection extends FeatureCollection<SimpleFeatureType,SimpleFeature>
- Modify SimpleFeatureSource to return SimpleFeatureCollection
- Update the GeoTools implementations to refer to the new user interface (literally search and replace)
public interface DataAccess<T,F> {
...
FeatureSource<T,F> getFeatureSource(Name)
T getSchema(Name);
}
public interface DataStore extends DataAccess<SimpleFeatureType, SimpleFeature> {
SimpleFeatureSource getFeatureSource(Name)
SimpleFeatureType getSchema( Name );
}
public interface SimpleFeatureSource extends FeatureSource<SimpleFeatureType,SimpleFeature>{
// changed api to type narrow to SimpleFeatureCollection
SimpleFeatureCollection getFeatures();
SimpleFeatureCollection getFeatures(Filter filter);
SimpleFeatureCollection getFeatures(Query);
}
public interface SimpleFeatureCollection extends FeatureCollection<SimpleFeatureType,SimpleFeature {
}
This is represented as simplefeaturecollection.patch - the creation of the patch went smoothly; in the few cases where a feature collection was actually generic I created a copy rather then take away a class usable by App schema.
- MaxFeaturesFeatureCollection copied to MaxSimpleFeatureCollection
- ...
Branch:
- contains both of the above patches
- based on email discussion SimpleFeatureIterator was added into the mix
This proposal is made on April 25th, given the nature of the patch I would like to move quickly on it.
To facilitate review a branch is created here:
Voting has started:
-
Andrea Aime +0 (concerned about schedule)
-
Ian Turton +1
-
Jody Garnett +1
-
| :white_check_mark: | :no_entry: | :warning: | :negative_squared_cross_mark: |
------------|--------------------|------------|-------------------------|-------------------------------| no progress | done | impeded | lack mandate/funds/time | volunteer needed |
-
✅ Introduce SimpleFeatureSource, SimpleFeatureCollection, SimpleFeatureIterator to trunk
- SimpleFeatureStore may wish to have modifyFeature methods that take a String rather then AttributeDescriptor or Name
-
❎ Update documentation examples/workbooks