Geometry should implement Comparable with generics

[aaime]

I've found out that sorting a list of GeoTools features by their Geometry causes an unchecked method invocation warning to be issued. E.g.:

List<SimpleFeature> features = ...;
featuresList.sort(Comparator.comparing(f -> (Geometry) f.getDefaultGeometry()));

Compiler warning details:

Unchecked method invocation: method comparing in interface Comparator is applied to given types
  required: Function<? super T,? extends U>
  found: Function<SimpleFeature,Geometry>
  where T,U are type-variables:
    T extends Object declared in method <T,U>comparing(Function<? super T,? extends U>)
    U extends Comparable<? super U> declared in method <T,U>comparing(Function<? super T,? extends U>)

The issue is due to Comparator.comparing signature:

    public static <T, U extends Comparable<? super U>> Comparator<T> comparing(
            Function<? super T, ? extends U> keyExtractor)

The result of the function should return a U extends Comparable<? super U>, which I guess is due to Geometry being a Comparable, rather than a Comparable<Geometry>.

[jnh5y]

Will this change break the API around Geometry? Basically, I'm wondering if we have to add this to the list for "JTS 2.0" or if it can be changed in a minor version bump?

[aaime]

Ideally, it should not, any code trying to compare against anything but a Geometry would receive a NPE. But it would cause a binary incompatibility for sure, since compareTo switches from compareTo(Object) to compareTo(Geometry) (and the type cast is not needed anymore).

[jnh5y]

Ok, if I'm following, sounds like it is only a plus for making the change?

[aaime]

Well, no, if there are subclasses implementing compareTo, outside of JTS, they will have to switch the argument from Object to Geometry. In GeoTools we have a few of those.

Mind, I'm not guaranteeing this will be a good move, it's something that I've observed (java compiler warnings are not allowed in GeoTools, had to suppress them), and started a conversation about it.