Implementing :units and :method as configurable options #95

Merged
merged 11 commits into from Apr 30, 2012

Conversation

Projects
None yet
6 participants

I've implemented the option to configure :units and :method to be used by Geocoder in order to solve the issue: #79.

A new configuration DSL and a configuration file generator were also included..

Now it's possible to do:

# Rails-like configurations - new configuration DSL
Geocoder.configure do
  config.units = :km
  config.method = :spherical
  config.language = 'pt-BR'
  # ...
end

# OR
Geocoder.configure.units = :km # Rails-like single configuration

# OR
Geocoder::Configurations.units = :km # Backward compatibility

# OR
class Model < ActiveRecord::Base
  geocoded_by :address, :units => :km # Per-model configuration
end

# OR
place.distance_to([0,0], :km) # Per-method specification

The generator rails g geocoder:config creates a new file config/initializers/geocoder.rb with all possible configurations commented out.

I've tried to maintain full backward compatibility, although I think in the future the International Units System could be used by default.

Some new tests were included to check the new functionalities, and all the older ones keep working.

Could you please consider include these commits in the new version of geocoder? I think it will be turn the gem a little bit more flexible end pragmatic.

Hi Alex, have you checked my solution for the units configuration?

Is there any pending issue?

Godisemo commented Nov 1, 2011

Bump! This seems to be really nice. I don't like miles since I live in europe and want to be able to configure this gem to use kilometers by default. Why has no one commented on this? Is it going to be pulled? When?

Thank you Godesimo, for checking out my proposal!

I haven't received no comments about this contribution, and, how it's relatively old, I don't know if there is compatibility with more recent changes in the gem.

I hope this code be useful yet...

drale2k commented Feb 11, 2012

This looks awesome, i need this. When will it be pulled?

simi commented Mar 20, 2012

Is this outdated now?

+1, i need this to use in Portugal, nice work.

I Abravalheri, i try your solution, but the app still show values in miles and not Km.

@skarface did you tried the version sent 8 months ago, or could you merge this pull request with a current version of alexreisner/geocoder?

When I've written this code, I'd mantained the default units to miles to maintain backward compatibilities. So would be necessary declare the units configuration to km in a initialization file... Was this your approach?

Have all the tests related with configurations passed??

I request your branch to test like this in my gem file,
gem 'geocoder', :git => 'git://github.com/abravalheri/geocoder.git', :ref => 'ff15ef2e7e4fed444939861132f43c54c3fd9e87'

But the unit still show me in miles, i test with code present in railscast ep 273

<% for location in @location.nearbys(15) %>

  • <%= link_to location.address, location %> (<%= location.distance.round(2) %>)
  • <% end %>

    I will try some tests to understand whats happen.

    @skarface Unfortunately, I can't run any ruby code right now. But when I go home I'll give a look at this problem!

    I use ruby 1.9.2 and Rails 3.2.2

    Sorry @skarface, but when I was investigating this issue, I could only conclude that location.distance still returns a value in km (if the Geocoder module was properly configurated in initializer file).

    I did the following test:

    # in terminal
    rails new test_geocoder
    
    # edit Gemfile:
    ## gem 'rails', '3.2.2'
    ## gem 'geocoder', :git => 'git://github.com/abravalheri/geocoder.git', :ref => 'ff15ef2e7e4fed444939861132f43c54c3fd9e87'
    
    bundle install
    
    rails g geocoder:config
    # uncomment the line 22 in config/initializers/geocoder.rb
    # change ":mi" to ":km" in the line 22 of config/initializers/geocoder.rb
    
    rails g model location name address latitude:float longitude:float
    rake db:create
    rake db:migrate
    # file app/models/location.rb
    class Location < ActiveRecord::Base
      geocoded_by :address
    end
    
    # file test/fixtures/locations.yml
     <% 10.times do |i| %>
     <%= "location_#{i}:"%>
      latitude: <%= -22.821124464287 + 0.01 * i %>
      longitude: <%= -47.074270248413 + 0.01 * i %>
      address: "R. Cícero Feltrim 58, Florianópolis - SC, Brasil"
    <% end %>
    
    #file test/unit/location_test.rb
    require 'test_helper'
    
    class LocationTest < ActiveSupport::TestCase
      def setup
        @origin = Location.first
        Geocoder.configure.units  = :km
      end
    
      test "location distance units with location#nearbys" do
        locations = @origin.nearbys(15)
    
        assert locations.length == Location.count - 1, <<-STR
          All locations in fixtures should be closer, so #{Location.count - 1} locations should be found.
          #{locations.length} found instead.
        STR
    
        locations.each do |location|
          puts "location.distance = #{location.distance}"
          puts "@origin.distance_to(location, :km) = #{@origin.distance_to(location, :km)}"
          puts "@origin.distance_to(location, :mi) = #{@origin.distance_to(location, :mi)}"
          assert (d1 = location.distance.round(2)) == (d2 = @origin.distance_to(location, :km).round(2)),
            "Distance from sql should be #{d2} km. #{d1} given instead."
        end
      end
    end

    By running rake test you should see that, inspite of the distance assertion fail, the value of location.distance is closest to @origin.distance_to(location, :km) then @origin.distance_to(location, :mi). Acctualy, I think the reason is an approximated method used in sqlite to calculate the radius when using nearbys.

    I don't know if in this test I've neglected something important to reproduce the failure pointed by your comment. So if you could share your files (or a similar project that reproduces this failure) in github, I will be happy to check it out for you.

    Maybe the problem is mine, i make some confusion. Many thanks.

    drale2k commented Apr 10, 2012

    I wonder why it is not being pulled

    @alexreisner alexreisner merged commit ff15ef2 into alexreisner:master Apr 30, 2012

    Owner

    alexreisner commented Apr 30, 2012

    This branch has been merged into master. Note that I renamed the :method option to :distances, which is more specific.

    Potential contributors: for a quicker response to pull requests, please limit them to one feature or bugfix, and name the request appropriately. This branch does add :units and :method as configurable options, but it also rewrites the entire configuration system, which makes it much more difficult to evaluate the quality and completeness of the code. In general, my response time increases exponentially with the size of the pull request.

    Thanks!

    simi commented Apr 30, 2012

    ❤️ @alexreisner

    +1

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment