kungfoo / geohash-java

Implementation of GeoHashes in java. We try to be/stay compliant to the spec, as far as possible.
Other
985 stars 309 forks source link

Added ability to expand bboxes to include points #45

Closed vinerich closed 2 years ago

vinerich commented 4 years ago

Implements #41

I think it should be good to go.

vinerich commented 4 years ago

Would be cool if you could release after merge. If you got no time atm that's not the matter. Would need it till the end of the month.

Thanks for your review! :)

kungfoo commented 4 years ago

I think foldIntoRange(value, min, max) would make most sense right now, judging from what it does.

On Fri, Aug 28, 2020, 08:31 Robin Weinreich notifications@github.com wrote:

@vinerich commented on this pull request.

In src/main/java/ch/hsr/geohash/util/DoubleUtil.java https://github.com/kungfoo/geohash-java/pull/45#discussion_r478858928:

@@ -0,0 +1,17 @@ +package ch.hsr.geohash.util; + +public class DoubleUtil { +

  • /**
    • Utility function to consistently compute the remainder over all java versions
    • @param value
    • @param remainder
    • @return
  • */
  • public static double remainderWithFix(double value, int remainder) {
  • double res = value % remainder;
  • // Fix for lower java versions, since the remainder-operator (%) changed in one version, idk which one

Hey @kungfoo https://github.com/kungfoo did you come up with a good name yet?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kungfoo/geohash-java/pull/45#discussion_r478858928, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKAKAWYNKCUT6ZDKZCULTSC5FN7ANCNFSM4NF7L4XA .

vinerich commented 4 years ago

Just a question, do you mean it like this?

public static double foldIntoRange(double value, double min, double max) {
    if(value < min)
        return min;
    else if (value > max)
        return max;
    else return value;
}

If yes, I'd prefer to just revert the function extraction back to the state before the commit.

The intent is indeed to clamp the value to (0,360).

I have to step back from my statement made here. That was short sighted and not correct. The following comparison

if (distanceEastToOtherEast <= distanceOtherWestToWest)

only works when the positive remainder is used to compute the distances. This has nothing to do with folding the value.

State before commit

double distanceEastToOtherEast = (other.eastLongitude - eastLongitude) % 360;
double distanceOtherWestToWest = (westLongitude - other.westLongitude) % 360;

if (distanceEastToOtherEast < 0)
    distanceEastToOtherEast += 360;
if (distanceOtherWestToWest < 0)
    distanceOtherWestToWest += 360;