From b27c2d291ed5b323b99efe766193ba2bc29b2bee Mon Sep 17 00:00:00 2001
From: Alex Reisner <alex@alexreisner.com>
Date: Tue, 5 Apr 2011 13:47:56 -0400
Subject: [PATCH] Change distance/bearing_between argument formats.

---
 lib/geocoder/calculations.rb | 63 +++++++++++++++++++++++++++---------
 lib/geocoder/orms/base.rb    | 36 ++++++++-------------
 test/geocoder_test.rb        | 15 ++++-----
 3 files changed, 67 insertions(+), 47 deletions(-)

diff --git a/lib/geocoder/calculations.rb b/lib/geocoder/calculations.rb
index 418d84d0..292b559b 100644
--- a/lib/geocoder/calculations.rb
+++ b/lib/geocoder/calculations.rb
@@ -38,24 +38,44 @@ module Geocoder
 
     ##
     # Distance between two points on Earth (Haversine formula).
-    # Takes two sets of coordinates and an options hash:
+    # Takes two points and an options hash.
+    # The points are given in the same way that points are given to all
+    # Geocoder methods that accept points as arguments. They can be:
+    #
+    # * an array of coordinates ([lat,lon])
+    # * a geocodable address (string)
+    # * a geocoded object (one which implements a +to_coordinates+ method
+    #   which returns a [lat,lon] array
+    #
+    # The options hash supports:
     #
     # * <tt>:units</tt> - <tt>:mi</tt> (default) or <tt>:km</tt>
     #
-    def distance_between(lat1, lon1, lat2, lon2, options = {})
+    def distance_between(point1, point2, options = {}, *args)
+      if args.size > 0
+        warn "DEPRECATION WARNING: Instead of passing lat1/lon1/lat2/lon2 as separate arguments to the distance_between method, please pass two two-element arrays: [#{point1},#{point2}], [#{options}, #{args.first}]. The old argument format will not be supported in Geocoder v.1.0."
+        point1 = [point1, point2]
+        point2 = [options, args.shift]
+        options = args.shift || {}
+      end
 
       # set default options
       options[:units] ||= :mi
 
+      # convert to coordinate arrays
+      point1 = extract_coordinates(point1)
+      point2 = extract_coordinates(point2)
+
       # convert degrees to radians
-      lat1, lon1, lat2, lon2 = to_radians(lat1, lon1, lat2, lon2)
+      point1 = to_radians(point1)
+      point2 = to_radians(point2)
 
       # compute deltas
-      dlat = lat2 - lat1
-      dlon = lon2 - lon1
+      dlat = point2[0] - point1[0]
+      dlon = point2[1] - point1[1]
 
-      a = (Math.sin(dlat / 2))**2 + Math.cos(lat1) *
-          (Math.sin(dlon / 2))**2 * Math.cos(lat2)
+      a = (Math.sin(dlat / 2))**2 + Math.cos(point1[0]) *
+          (Math.sin(dlon / 2))**2 * Math.cos(point2[0])
       c = 2 * Math.atan2( Math.sqrt(a), Math.sqrt(1-a))
       c * earth_radius(options[:units])
     end
@@ -64,7 +84,8 @@ module Geocoder
     # Bearing between two points on Earth.
     # Returns a number of degrees from due north (clockwise).
     #
-    # Also accepts an options hash:
+    # See Geocoder::Calculations.distance_between for
+    # ways of specifying the points. Also accepts an options hash:
     #
     # * <tt>:method</tt> - <tt>:linear</tt> (default) or <tt>:spherical</tt>;
     #   the spherical method is "correct" in that it returns the shortest path
@@ -74,15 +95,27 @@ module Geocoder
     #
     # Based on: http://www.movable-type.co.uk/scripts/latlong.html
     #
-    def bearing_between(lat1, lon1, lat2, lon2, options = {})
+    def bearing_between(point1, point2, options = {}, *args)
+      if args.size > 0
+        warn "DEPRECATION WARNING: Instead of passing lat1/lon1/lat2/lon2 as separate arguments to the bearing_between method, please pass two two-element arrays: [#{point1},#{point2}], [#{options}, #{args.first}]. The old argument format will not be supported in Geocoder v.1.0."
+        point1 = [point1, point2]
+        point2 = [options, args.shift]
+        options = args.shift || {}
+      end
+
       options[:method] = :linear unless options[:method] == :spherical
 
+      # convert to coordinate arrays
+      point1 = extract_coordinates(point1)
+      point2 = extract_coordinates(point2)
+
       # convert degrees to radians
-      lat1, lon1, lat2, lon2 = to_radians(lat1, lon1, lat2, lon2)
+      point1 = to_radians(point1)
+      point2 = to_radians(point2)
 
       # compute deltas
-      dlat = lat2 - lat1
-      dlon = lon2 - lon1
+      dlat = point2[0] - point1[0]
+      dlon = point2[1] - point1[1]
 
       case options[:method]
       when :linear
@@ -90,9 +123,9 @@ module Geocoder
         x = dlat
 
       when :spherical
-        y = Math.sin(dlon) * Math.cos(lat2)
-        x = Math.cos(lat1) * Math.sin(lat2) -
-            Math.sin(lat1) * Math.cos(lat2) * Math.cos(dlon)
+        y = Math.sin(dlon) * Math.cos(point2[0])
+        x = Math.cos(point1[0]) * Math.sin(point2[0]) -
+            Math.sin(point1[0]) * Math.cos(point2[0]) * Math.cos(dlon)
       end
 
       bearing = Math.atan2(x,y)
diff --git a/lib/geocoder/orms/base.rb b/lib/geocoder/orms/base.rb
index 17346e68..1b98ae8e 100644
--- a/lib/geocoder/orms/base.rb
+++ b/lib/geocoder/orms/base.rb
@@ -18,52 +18,42 @@ module Geocoder
 
       ##
       # Calculate the distance from the object to an arbitrary point.
-      # The point can be:
-      #
-      # * an array of coordinates ([lat,lon])
-      # * a geocoded object (one which implements a +to_coordinates+ method
-      #   which returns a [lat,lon] array
-      # * a geocodable address (string)
-      #
-      # Also takes a symbol specifying the units (:mi or :km; default is :mi).
+      # See Geocoder::Calculations.distance_between for ways of specifying
+      # the point. Also takes a symbol specifying the units
+      # (:mi or :km; default is :mi).
       #
       def distance_to(point, *args)
         if point.is_a?(Numeric) and args[0].is_a?(Numeric)
           warn "DEPRECATION WARNING: Instead of passing latitude/longitude as separate arguments to the distance_to/from method, please pass an array [#{point},#{args[0]}], a geocoded object, or a geocodable address (string). The old argument format will not be supported in Geocoder v.1.0."
+          point = [point, args.shift]
         end
         return nil unless geocoded?
-        units = args.last.is_a?(Symbol) ? args.pop : :mi
-        them = args.size > 0 ? [point, args.first] :
-          Geocoder::Calculations.extract_coordinates(point)
-        us = to_coordinates
         Geocoder::Calculations.distance_between(
-          us[0], us[1], them[0], them[1], :units => units)
+          to_coordinates, point, :units => args.pop || :mi)
       end
 
       alias_method :distance_from, :distance_to
 
       ##
       # Calculate the bearing from the object to another point.
-      # See distance_to for various ways to specify the point.
+      # See Geocoder::Calculations.distance_between for
+      # ways of specifying the point.
       #
       def bearing_to(point, options = {})
-        return nil unless geocoded? &&
-          them = Geocoder::Calculations.extract_coordinates(point)
-        us = to_coordinates
+        return nil unless geocoded?
         Geocoder::Calculations.bearing_between(
-          us[0], us[1], them[0], them[1], options)
+          to_coordinates, point, options)
       end
 
       ##
       # Calculate the bearing from another point to the object.
-      # See distance_to for various ways to specify the point.
+      # See Geocoder::Calculations.distance_between for
+      # ways of specifying the point.
       #
       def bearing_from(point, options = {})
-        return nil unless geocoded? &&
-          them = Geocoder::Calculations.extract_coordinates(point)
-        us = to_coordinates
+        return nil unless geocoded?
         Geocoder::Calculations.bearing_between(
-          them[0], them[1], us[0], us[1], options)
+          point, to_coordinates, options)
       end
 
       ##
diff --git a/test/geocoder_test.rb b/test/geocoder_test.rb
index d705810a..3cc67138 100644
--- a/test/geocoder_test.rb
+++ b/test/geocoder_test.rb
@@ -160,14 +160,14 @@ class GeocoderTest < Test::Unit::TestCase
   end
 
   def test_distance_between_in_miles
-    assert_equal 69, Geocoder::Calculations.distance_between(0,0, 0,1).round
-    la_to_ny = Geocoder::Calculations.distance_between(34.05,-118.25, 40.72,-74).round
+    assert_equal 69, Geocoder::Calculations.distance_between([0,0], [0,1]).round
+    la_to_ny = Geocoder::Calculations.distance_between([34.05,-118.25], [40.72,-74]).round
     assert (la_to_ny - 2444).abs < 10
   end
 
   def test_distance_between_in_kilometers
-    assert_equal 111, Geocoder::Calculations.distance_between(0,0, 0,1, :units => :km).round
-    la_to_ny = Geocoder::Calculations.distance_between(34.05,-118.25, 40.72,-74, :units => :km).round
+    assert_equal 111, Geocoder::Calculations.distance_between([0,0], [0,1], :units => :km).round
+    la_to_ny = Geocoder::Calculations.distance_between([34.05,-118.25], [40.72,-74], :units => :km).round
     assert (la_to_ny - 3942).abs < 10
   end
 
@@ -237,11 +237,8 @@ class GeocoderTest < Test::Unit::TestCase
     methods.each do |m|
       directions.each_with_index do |d,i|
         opp = directions[(i + 2) % 4] # opposite direction
-        p1 = points[d]
-        p2 = points[opp]
-
-        args = p1 + p2 + [:method => m]
-        b = Geocoder::Calculations.bearing_between(*args)
+        b = Geocoder::Calculations.bearing_between(
+          points[d], points[opp], :method => m)
         assert (b - bearings[opp]).abs < 1,
           "Bearing (#{m}) should be close to #{bearings[opp]} but was #{b}."
       end
-- 
GitLab