search

wdtinc / mapbox-vector-tile-java

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

TagKeyValueMapConverter and order #9

Closed snodnipper closed 6 years ago

snodnipper commented 6 years ago

From working with the library, the TagKeyValueMapConverter does not preserve the ordering of attributes.

It would be great if we could use, say, a LinkedHashMap so that attributes look consistent.

For example, geo analysts like to see:

id: 123456789
name: Paul
age: 47

without the order of the above being arbitrary.

For tests I created a TagKeyValueMapConverterOrder class but perhaps the overhead isn't so bad for default use?

https://github.com/wdtinc/mapbox-vector-tile-java/blob/6865a1654d8d21256f2ce47ca13b9af66e3054b2/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/TagKeyValueMapConverter.java#L71

ShibaBandit commented 6 years ago

Good link. I think I'd need to swap the ID placement into the map above the other properties as well instead of at the end of the function to get the desired effect? Let me think about a solution here, in the between time you can copy that class into your own that implements the ITagConverter interface.

snodnipper commented 6 years ago

I have had a quick look over with @AndrewRadburn.

We thought it might be easier if I introduce a Feature class that contains JTS geometry.

Specifically, the feature would contain an ID of Java type Long at the class level as per specification, which may be null.

We then believe we can get rid of the id parameter from toUserData as any user attribute with an ID might be completely independent (e.g. fid, toid for feature ID or topological ID etc.). https://github.com/wdtinc/mapbox-vector-tile-java/blob/6865a1654d8d21256f2ce47ca13b9af66e3054b2/src/main/java/com/wdtinc/mapbox_vector_tile/adapt/jts/TagKeyValueMapConverter.java#L62

The structure would therefore be:

The difference from Protobuf generated code is that the above would be easy to work with and has JTS geometries. Feature.getAttributes could, for instance, be redirected to Geometry.getUserData to maintain backwards compatibility for the moment.

LinkedHashMap would therefore satisfy the contract of keys being ordered according to insertion.

How does that sound to you?

If ok, I could update https://github.com/wdtinc/mapbox-vector-tile-java/pull/8 with the Feature class.

ShibaBandit commented 6 years ago

So the purpose of the com.wdtinc.mapbox_vector_tile.adapt.jts package is to translate JTS to/from MVT data. I think the interest in receiving ordered maps out of a MVT into JTS objects is something easy for me to grasp and something that I can put built-in support for. I'll comment on the layer/ feature / layergroup ideas in your pull request.

ShibaBandit commented 6 years ago

Merged via #10

ShibaBandit commented 6 years ago

Available in 2.0.0 in maven central.