LatLong.compareTo() fix by Sublimis · Pull Request #1628 · mapsforge/mapsforge
Conversation
LatLong.compareTo() was incorrectly implemented, so it could happen that e.g. latLon1.compareTo(latLon2) == latLon2.compareTo(latLon1) == 1 for coordinates latLon1=(1, 0) and latLon2=(0, 1).
Although the fix is crucial, LatLong.compareTo() does not seem to be used at the moment (is this correct?), so it may not change any current behavior of the library.
@Sublimis Do you think we need some tests in LatLongTest class?
@Sublimis Do you think we need some tests in
LatLongTestclass?
This could be useful, yes. Some tests will be crafted.
It's interesting to note that if LatLong were to extend from org.mapsforge.core.model.Point, and encapsulate the fields with getters/setters, we'd have all this automatically :)
LatLong comparison looks similar to the Point comparison,
except that Point compares the x / longitude coordinate first.
@Sublimis Should we compare latitude first in LatLong?
@devemux86 This probably wouldn't matter while running, but if you think it improves consistency, that's good. It's interesting to note how the latitude always comes first, it's always latlon, not lonlat.
What do you think of this cleanup now?
It's interesting to note how the latitude always comes first, it's always latlon, not lonlat.
While it's usually x,y, coordinates in mapping are often latitude,longitude.
What do you think of this cleanup now?
@Sublimis Looks good, we can merge it if it's ready.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters