# Haversine formula fix #2408

Merged
merged 3 commits into from Oct 5, 2017

## Conversation

Projects
None yet
3 participants
Member

### pomadchin commented Oct 5, 2017

 Fixes #2383
``` Use a different Haversine formula ```
``` 550ff49 ```

### pomadchin requested a review from echeipeshOct 5, 2017

``` Add HaversineSpec (not very accurate); fixed ZoomedLayoutSchemeSpec ```
``` 730ea96 ```

### metasim requested changes Oct 5, 2017

 package geotrellis.util object Haversine { val EARTH_RADIUS = 6378137d // Use what gdal2tiles uses.

#### metasim Oct 5, 2017

Member

I'd suggest adding a doc comment declaring the units used.

 val EARTH_RADIUS = 6378137d // Use what gdal2tiles uses. // (x: Double, y: Double) points def apply(start: (Double, Double), end: (Double, Double), R: Double = EARTH_RADIUS): Double = {

#### metasim Oct 5, 2017

Member

For future GeoTrellisites, I'd suggest a doc comment indicating what the components of `start` and `end` are. e.g. (Latitude, Longitude), or (Longitude, Latitude) or (radius, angle), etc.

 @@ -31,6 +31,9 @@ object Point { implicit def jtsCoord2Point(coord: jts.Coordinate): Point = Point(factory.createPoint(coord)) implicit def pointToTuple2(point: Point): (Double, Double) =

#### metasim Oct 5, 2017

Member

While this is a nice convenience for people familiar with the particular code that translates between `Point` and `Tuple2`, I've been burned enough by unexpected or unintended conversions in the past to avoid this sort of thing personally, especially when the conversion is to a more general type. If you were converting between `Point` and something like `Position(lat, lng)` I'd be a bit less concerned because the translation semantics of `x` -> `lat` and `y` -> `lng` are clear, but `_1` and `_2` have no inherent semantics and therefore information is lost in the implicit conversion.

Member

Yes, i decided to remove this implicit at all as it is indeed ambiguous.

Member

### metasim approved these changes Oct 5, 2017

``` Satisfy Simeons comments ```
``` e28762f ```

### echeipesh merged commit `cdaa151` into locationtech:master Oct 5, 2017 2 checks passed

#### 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details