wdtinc / mapbox-vector-tile-java

Java Mapbox Vector Tile Library for Encoding/Decoding
Apache License 2.0
147 stars 73 forks source link

Android pre API 24 language support #13

Closed snodnipper closed 6 years ago

snodnipper commented 6 years ago

I have spent the day running code quality tools to get this going on old versions of Android. I can feedback as a separate issue - I needed Java 1.7 API compatibility, for instance.

Anyhow, these lines fall through and look suspicious.

https://github.com/wdtinc/mapbox-vector-tile-java/blob/fc61116063e6b9fa072a26cd73146920edd0e473/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/JtsAdapter.java#L445

https://github.com/wdtinc/mapbox-vector-tile-java/blob/fc61116063e6b9fa072a26cd73146920edd0e473/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/JtsAdapter.java#L517

https://github.com/wdtinc/mapbox-vector-tile-java/blob/master/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/MvtReader.java#L535

https://github.com/wdtinc/mapbox-vector-tile-java/blob/master/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/MvtReader.java#L592

ShibaBandit commented 6 years ago

I haven't done much android in a while, but I heard they support java 8 via desugar and deprecated Jack? Would this be valid on old android versions? How far back are you needing to go? https://developer.android.com/studio/write/java8-support.html

https://developer.android.com/reference/java/util/Collections.html#emptyList() https://developer.android.com/reference/java/util/List.html#toArray()

snodnipper commented 6 years ago

So we have Java 8 syntax support but the APIs are not (currently) ported back beyond API 24, which is Android 7.0 (Nougat).

https://developer.android.com/studio/write/java8-support.html#supported_features

I have altered the wdtinc code for the moment in our vt-support library.

You can get a rough idea of what has changed in my repo.

I need to get a demo working for tomorrow (I have not got this fresh code running yet) so I'll feedback any issues.

Happy to help as necessary (e.g. PR Java 1.7 code, add CircleCI config if you'd like CI deployments in the project etc.)

snodnipper commented 6 years ago
-    final Integer mapIndex = keys.putIfAbsent(key, nextIndex);
+    // Android API 24 call
+    // final Integer mapIndex = keys.putIfAbsent(key, nextIndex);
+    final Integer mapIndex;
+    if (!vals.containsKey(key)) {
+      mapIndex = vals.put(key, nextIndex);
+    }
+    else {
+      mapIndex = vals.get(key);
+    }

Minor things like this. It is not pretty but is documented. https://developer.android.com/reference/java/util/Map.html#putIfAbsent(K, V)

It certainly isn't a pleasurable experience...because usually Java version defines allowed syntax and methods that can be called.

Continuing...

ShibaBandit commented 6 years ago

Added travis ci to run 'mvn clean test' to master.

ShibaBandit commented 6 years ago

Created branch 'android_api_24_support'

snodnipper commented 6 years ago

Sweet - I'll commit the changes currently visible in our vt-support lib and start a PR.

ShibaBandit commented 6 years ago

Added bug label since several issues were picked up by findbugs.

ShibaBandit commented 6 years ago

@snodnipper Since you've tested with API 21 mentioned in #14 I may go ahead and update the readme to reflect that.

ShibaBandit commented 6 years ago

Closed via #14 and #16

snodnipper commented 6 years ago

Thanks @ShibaBandit and folks for commenting etc. to help push this over the line.

If anyone wants to talk about functionality etc. I am happy to talk on the http://thespatialcommunity.org/ slack channel - e.g. very old Android support etc.

👍