GeoTools
  1. GeoTools
  2. GEOT-3051

SimpleFeatureCollection to remove generics from example code

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6.3
    • Fix Version/s: 2.7-M0
    • Component/s: data
    • Labels:
      None

      Description

      I have had the pleasure of running through the geotools training material a couple times in the last month; and I have watched each person get stuck in the same spot.

      Generics.

      Generics in the Csv2Shape.java class.

      public static void main(String[] args) throws Exception {
         ...
         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);
         }
      }
      

      For now the solution (committed) is to use:

      @SuppressWarnings("unchecked")
      public static void main(String[] args) throws Exception {
         ...
         String typeName = newDataStore.getTypeNames()[0];
         FeatureSource featureSource = newDataStore.getFeatureSource(typeName);
      
         if (featureSource instanceof FeatureStore) {
             FeatureStore featureStore = (FeatureStore) featureSource;
             ...
         }
         } else {
             System.out.println(typeName + " does not support read/write access");
             System.exit(1);
         }
      }
      

      In addition to being magic to beginning Java users generics make the lines so long it no longer looks like a simple assignment; and by the time they have read word for word through the cast to feature store they need to start again to see what happened.

      Previous attempts using FeatureSource<?,?> were even more scary; they did not even try reading the code.

      So here is the minimal idea for the Csv2SShape example:

      • no new methods
      • SimpleFeatureCollection extends FeatureCollection<SimpleFeatureType,SimpleFeature>
      • SimpleFeatureSource and SimpleFeatureStore once again with no new API; just returning SimpleFeatureCollection

      This task is done when:

      • casts are no longer needed example

      I am prepared for this to be a change only on trunk; but I would need some assurances I will have a release of trunk for FOSS4G this year.

      Update


      Here is what the change would look like:

      public static void main(String[] args) throws Exception {
         ...
         String typeName = newDataStore.getTypeNames()[0];
         SimpleFeatureSource featureSource = newDataStore.getFeatureSource(typeName);
      
         if (featureSource instanceof SimpleFeatureStore) {
             SimpleFeatureStorefeatureStore = (SimpleFeatureStore) featureSource;
             ...
         }
         } else {
             System.out.println(typeName + " does not support read/write access");
             System.exit(1);
         }
      }
      
      1. simplefeaturecollection.patch
        1.17 MB
        Jody Garnett
      2. simplefeaturesource.patch
        43 kB
        Jody Garnett

        Activity

        Hide
        Michael Bedward added a comment -
        I think SimpleFeatureCollection and SimpleFeatureSource would be great Jody. Definitely make the example look a lot cleaner and less intimidating.
        Show
        Michael Bedward added a comment - I think SimpleFeatureCollection and SimpleFeatureSource would be great Jody. Definitely make the example look a lot cleaner and less intimidating.
        Hide
        Jody Garnett added a comment -
        Okay tried my first attempt at a patch today.

        Started by going through DataStore api and returning SimpleFeatureSource, SimpleFeatureReader and SimpleFeatureWriter to get rid of the generics through type narrowing. The thinking is that all existing assignments coming out of DataStore methods will still work.

        Here is what actually happened: SimpleFeatureReader and SimpleFeatureWriter "rippled" through the implementations causing breaks; many where code was implementing FeatureReader<SimpleFeatureType,SimpleFeature> could just be updated; but there was around 20% that actually stayed with the generic <T,F> notation. I implemented a few EmptySimpleFeatureReaders and EmptySimpleFeatureWriters to see where that would go.

        Next up I will try *just* introducing SimpleFeatureSource and SimpleFeatureStore - since that is the source of most of the casts in client code.
        Show
        Jody Garnett added a comment - Okay tried my first attempt at a patch today. Started by going through DataStore api and returning SimpleFeatureSource, SimpleFeatureReader and SimpleFeatureWriter to get rid of the generics through type narrowing. The thinking is that all existing assignments coming out of DataStore methods will still work. Here is what actually happened: SimpleFeatureReader and SimpleFeatureWriter "rippled" through the implementations causing breaks; many where code was implementing FeatureReader<SimpleFeatureType,SimpleFeature> could just be updated; but there was around 20% that actually stayed with the generic <T,F> notation. I implemented a few EmptySimpleFeatureReaders and EmptySimpleFeatureWriters to see where that would go. Next up I will try *just* introducing SimpleFeatureSource and SimpleFeatureStore - since that is the source of most of the casts in client code.
        Hide
        Jody Garnett added a comment -
        Introduce SimpleFeatureSource and fix any compile errors; this removes a lot of the casts. Next step is to hook in SimpleFeatureCollection
        Show
        Jody Garnett added a comment - Introduce SimpleFeatureSource and fix any compile errors; this removes a lot of the casts. Next step is to hook in SimpleFeatureCollection
        Hide
        Jody Garnett added a comment -
        Okay this is the big one.
        Show
        Jody Garnett added a comment - Okay this is the big one.
        Hide
        Jody Garnett added a comment -
        Now passes tests; had a cast exception in renderer. Pretty happy with this patch; only rough spot was in renderer where I wished the code would handle a generic feature collection.
        Show
        Jody Garnett added a comment - Now passes tests; had a cast exception in renderer. Pretty happy with this patch; only rough spot was in renderer where I wished the code would handle a generic feature collection.
        Hide
        Jody Garnett added a comment - - edited
        Branch available:http://svn.osgeo.org/geotools/branches/simple-features Full maven clean install works on both Java 5 and Java 6
        Show
        Jody Garnett added a comment - - edited Branch available: http://svn.osgeo.org/geotools/branches/simple-features Full maven clean install works on both Java 5 and Java 6
        Hide
        Ben Caradoc-Davies added a comment -
        Committed a fix for gt-webservice WS_DataStore in r35327.
        Show
        Ben Caradoc-Davies added a comment - Committed a fix for gt-webservice WS_DataStore in r35327.
        Hide
        Ben Caradoc-Davies added a comment -
        Assigned to Jody as he is doing the work. ;-)
        Show
        Ben Caradoc-Davies added a comment - Assigned to Jody as he is doing the work. ;-)
        Hide
        Jody Garnett added a comment -
        Was applied over the weekend; thanks for the hard work everyone.
        Show
        Jody Garnett added a comment - Was applied over the weekend; thanks for the hard work everyone.

          People

          • Assignee:
            Jody Garnett
            Reporter:
            Jody Garnett
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: