opendatalab-de / geojson-jackson

GeoJson POJOs for Jackson - serialize and deserialize objects with ease
http://blog.opendatalab.de
Apache License 2.0
263 stars 94 forks source link

Support for additional elements in coordinate #27

Closed notanyron closed 8 years ago

notanyron commented 8 years ago

The Positions portion of the GeoJson specification allows for more than three elements. This change optionally allows an arbitrary number of elements after altitude.

grundid commented 8 years ago

Thanks for pointing this out and the pull request. I guess you have put more thought into this, but I was just thinking if it might be easier to store all values in the LngLatAlt class in an array instead of three predefined values and then an array with additional values. Have you thought about such an option?

notanyron commented 8 years ago

To be honest I had not thought of this. I definitely did not want to break backward compatibility, so I don't think it's reasonable to remove the get/set methods. It would be straightforward to back them with a single array, however, leaving the public interface consistent. I'd be happy to make that change if you think that's easier.

notanyron commented 8 years ago

I gave this some more thought, and implementing lng, lat and alt as elements of the array is trickier than it might first seem. Consider the following case:

LngLatAlt lla = new LngLatAlt(); lla.setLongitude(42); // We don't know how big to make the array yet... lla.setLatitude(43); // Still don't know lla.setAltitude(44); // Still don't know. We might be done, or we might have more elements.

To implement this correctly using an array means that we would have to start with an array of at least 2 elements with the assumption that we'll have at least lng and lat. When we call setAltitude, we would have to copy them into a size 3 array. If setAdditionalElements is called, we would have to copy again.

While writing the implementation in the pull request, I started with an ArrayList, which would solve the above problems. But, this introduces a number of issues where a Double instance can be null. I settled on double[] for this reason.

Please let me know if you see a clean way around these issues. My current thought is to leave the implementation as is.

grundid commented 8 years ago

Thanks for checking out other possibilities. I also think your implementation is the right solution.

notanyron commented 8 years ago

Thanks for the merge!