HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Haversine Formula in Clojure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
formulahaversineclojure

Problem

I implemented the Haversine formula to calculate the distance between two (latitude, longitude) coordinates.

I was wondering if it looks natural to Clojure programmers. What could be improved?

(ns br.com.reactive-poc.calc)

(def raio 6372.795477598)

(defn calculate-distance
"Calcula distância entre duas coordenadas geográficas"
([source to]
  {:pre [every? (every-pred map? #(contains? % :lgt) #(contains? % :ltd)) [source to]]}
  (let [delta-latitude (Math/toRadians (- (:source/ltd source) (:to/ltd to)))
        delta-longitude (Math/toRadians (- (:source/lgt source) (:to/lgt to)))
        ]
    (* (* raio (* 2 (Math/atan2 (Math/sqrt (+ (Math/pow (Math/sin (/ delta-latitude 2)) 2) 
                                              (* (Math/cos (Math/toRadians (:source/ltd source))) (Math/cos (Math/toRadians (:to/ltd to))) (Math/sin (/ delta-longitude 2)) (Math/sin (/ delta-longitude 2))))) 
                                (Math/sqrt (- 1 (+ (Math/pow (Math/sin (/ delta-latitude 2)) 2) 
                                                   (* (Math/cos (Math/toRadians (:source/ltd source))) (Math/cos (Math/toRadians (:to/ltd to)) ) (Math/sin (/ delta-longitude 2)) (Math/sin (/ delta-longitude 2))))))))) 1000)
    )
  )
)

Solution

Naming and interface

Please avoid mixing English (calculate-distance) with Portuguese (raio). The code should be in English; write your comments in whatever language you prefer.

ltd and lgt seem like awkward abbreviations; lat and lon or lat and lng seem more conventional. The latter is what Google uses for the Maps and Android APIs, for example.

Putting "calculate" in the name of a function seems redundant, especially in a functional programming language.

One of the most surprising behaviours is that your function gives the result in meters, even though your raio constant is in kilometers. To discover that quirk, you would have to scroll all the way to column 219 at the end of the function, at the end of a very long line of code! A better comment on the function would help; better naming would be beneficial as well.

Implementation

As I mentioned, those are very long lines of code — almost unreadable. It would be more readable if you broke down the calculation into subexpressions.

In fact, (+ (Math/pow (Math/sin (/ delta-latitude 2)) 2) (* (Math/cos (Math/toRadians (:source/ltd source))) (Math/cos (Math/toRadians (:to/ltd to))) (Math/sin (/ delta-longitude 2)) (Math/sin (/ delta-longitude 2)))) is a common subexpression that you evaluate twice. So, not only is your code unreadable, it's also inefficient.

I would convert the latitudes and longitudes to radians right away, since degrees are useless for trigonometry. Half the difference in angles are easily recognizable quantities. Defining the \$\sin^2(\theta)\$ function as a lambda is also useful.

(def earth-radius-km 6372.795477598)

(defn distance
    "Calcula distância entre duas coordenadas geográficas em metros"
    ([latlng1 latlng2]
        {:pre [every? (every-pred map? #(contains? % :lat) #(contains? % :lng)) [latlng1 latlng2]]}
        (let [lat1 (Math/toRadians (:latlng1/lat latlng1))
              lng1 (Math/toRadians (:latlng1/lng lnglng1))
              lat2 (Math/toRadians (:latlng2/lat latlng2))
              lng2 (Math/toRadians (:latlng2/lng lnglng2))
              half-dlat (/ (- lat2 lat1) 2)
              half-dlng (/ (- lng2 lng1) 2)
              sin2 (fn [x] (Math/pow (Math/sin x) 2))
              a (+ (sin2 half-dlat) (* (Math/cos lat1) (Math/cos lat2) (sin2 half-dlng)))]
             (* 2000 earth-radius-km (Math/atan2 (Math/sqrt a) (Math/sqrt (- 1 a)))))))

Code Snippets

(def earth-radius-km 6372.795477598)

(defn distance
    "Calcula distância entre duas coordenadas geográficas em metros"
    ([latlng1 latlng2]
        {:pre [every? (every-pred map? #(contains? % :lat) #(contains? % :lng)) [latlng1 latlng2]]}
        (let [lat1 (Math/toRadians (:latlng1/lat latlng1))
              lng1 (Math/toRadians (:latlng1/lng lnglng1))
              lat2 (Math/toRadians (:latlng2/lat latlng2))
              lng2 (Math/toRadians (:latlng2/lng lnglng2))
              half-dlat (/ (- lat2 lat1) 2)
              half-dlng (/ (- lng2 lng1) 2)
              sin2 (fn [x] (Math/pow (Math/sin x) 2))
              a (+ (sin2 half-dlat) (* (Math/cos lat1) (Math/cos lat2) (sin2 half-dlng)))]
             (* 2000 earth-radius-km (Math/atan2 (Math/sqrt a) (Math/sqrt (- 1 a)))))))

Context

StackExchange Code Review Q#156722, answer score: 5

Revisions (0)

No revisions yet.