Cleaned up code with a lot of "extract method" Ruby refactorings in particular - mongoid spec passes :)

[kristianmandrup]

$ bspec spec/models/place_spec.rb 
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
.

Finished in 0.02163 seconds
1 example, 0 failures

[apneadiving]

I readapted code here: https://github.com/apneadiving/Google-Maps-for-Rails/tree/mongoid_adapter

But your Mongoid spec is failing (even if I comment gmaps4rails related code )

 1) Gmaps4rails::ActsAsGmappable standard configuration, valid place should save longitude and latitude to the customized position array
     Failure/Error: let(:place)         { Factory(:place) }
     NoMethodError:
     undefined method `[]' for nil:NilClass
     # ./spec/models/place_spec.rb:26:in `block (2 levels) in <top (required)>'
     # ./spec/models/place_spec.rb:37:in `block (3 levels) in <top (required)>'

Could you have a look?

[apneadiving]

Sidenote: I removed almost all your refactorings.

I agree "extract method" is a good practice but it's should be done when there is added value and it's not really the case when you replace one-liners.

Actually, methods known from everyone (like send) become hidden by your own methods which finally increase complexity.

Look at the code now: it's shorter than yours and (I feel like) it's clearer.

[kristianmandrup]

Well, when I did those refactorings the code became much clearer to me. I have no problem with u cleaning it up even more in a way that suits you :) will the new feature make it into next release? Thanks!

[apneadiving]

As i wrote, the spec fails for me :(

That's why I'd like you to have a look at my branch (moreover, the bug doesn't seem to be linked to the gem's code bit only with Mongoid).

I'll release once the situation is clear.

I'll release once

Sent from my iPhone

On 6 août 2012, at 08:35, Kristian Mandrupreply@reply.github.com wrote:

Well, when I did those refactorings the code became much clearer to me. I have no problem with u cleaning it up even more in a way that suits you :) will the new feature make it into next release? Thanks!

---

Reply to this email directly or view it on GitHub: https://github.com/apneadiving/Google-Maps-for-Rails/pull/245#issuecomment-7517521

[kristianmandrup]

OK, I will have a look at it. The Mongoid spec passes on my branch.

[apneadiving]

Yes I know... I don't think I changed anything regarding Mongoid. Let me know :)

Sent from my iPhone

On 6 août 2012, at 11:00, Kristian Mandrupreply@reply.github.com wrote:

OK, I will have a look at it. The Mongoid spec passes on my branch.

---

Reply to this email directly or view it on GitHub: https://github.com/apneadiving/Google-Maps-for-Rails/pull/245#issuecomment-7519583

[kristianmandrup]

The spec passes when you insert the following into place.rb:

  include Gmaps4rails::ActsAsGmappable

  acts_as_gmappable :address => :address, :position => :pos

Not sure why you removed it and expected place.pos.should_not be_nil?

 bundle exec rspec spec/models/place_spec.rb 
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
.

Finished in 0.02457 seconds
1 example, 0 failures

[apneadiving]

Actually, the spec fails when place is created.

I thought it was related to the gem's code so I removed any reference to it.

I added the error message in a previous post.

Sent from my iPhone

On 6 août 2012, at 11:13, Kristian Mandrupreply@reply.github.com wrote:

The spec passes when you insert the following into place.rb:

 include Gmaps4rails::ActsAsGmappable

 acts_as_gmappable :address => :address, :position => :pos

Not sure why you removed it and expected place.pos.should_not be_nil?

bundle exec rspec spec/models/place_spec.rb 
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
.

Finished in 0.02457 seconds
1 example, 0 failures

---

Reply to this email directly or view it on GitHub: https://github.com/apneadiving/Google-Maps-for-Rails/pull/245#issuecomment-7519815

[apneadiving]

From: /Users/benjaminroth/Code/dev_plugin/Google-Maps-for-Rails/spec/models/place_spec.rb @ line 36:

  31:   end
  32:   
  33:   context "standard configuration, valid place" do
  34:     it "should save longitude and latitude to the customized position array" do
  35:       # set_gmaps4rails_options!(:position  => 'location')
  => 36:       binding.pry
  37:       place.pos.should_not be_nil
  38:       place.should have_same_position_as TOULON
  39:     end
  40:   end
  41: end

[1] pry(#<RSpec::Core::ExampleGroup::Nested_1::Nested_1>)> place
NoMethodError: undefined method `[]' for nil:NilClass
from /Users/benjaminroth/.rvm/gems/ruby-1.9.2-p290/gems/moped-1.2.0/lib/moped/node.rb:74:in `block in command'

[kristianmandrup]

Must be a configuration issue on your side!?

$ bundle exec rspec spec/models/place_spec.rb 
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

From: /Users/kmandrup/private/repos/try/Google-Maps-for-Rails/spec/models/place_spec.rb @ line 36:

    31:   end
    32:   
    33:   context "standard configuration, valid place" do
    34:     it "should save longitude and latitude to the customized position array" do
    35:       # set_gmaps4rails_options!(:position  => 'location')
 => 36:       binding.pry
    37:       place.pos.should_not be_nil
    38:       place.should have_same_position_as TOULON
    39:     end
    40:   end
    41: end

[1] pry(#<RSpec::Core::ExampleGroup::Nested_1::Nested_1>)> place
=> #<Place _id: 501f8fc4ff096145d8000001, _type: nil, pos: [43.124228, 5.928], address: "Toulon, France", gmaps: true>

Works fine for me :)

$ mongod
mongod --help for help and startup options
Mon Aug  6 11:34:00 [initandlisten] MongoDB starting : pid=55335 port=27017 dbpath=/data/db/ 64-bit host=Kristian-Mandrups-MacBook-Air.local
Mon Aug  6 11:34:00 [initandlisten] db version v2.0.2, pdfile version 4.5

[apneadiving]

Well:

➜  ~ mongod       
mongod --help for help and startup options
Mon Aug  6 11:37:23 [initandlisten] MongoDB starting : pid=28669 port=27017 dbpath=/data/db/ 64-bit host=Benjamin-Roths-MacBook-Pro.local
Mon Aug  6 11:37:23 [initandlisten] db version v2.0.5, pdfile version 4.5

What version of Ruby do you run?

[kristianmandrup]

1.9.3-p125

[apneadiving]

Ok was due to Ruby version. Wanna release today.

[apneadiving]

Had to make some conditionals to have the specs passing several versions of Ruby. This merge took much time but.. it's done.

Thanks for your work.

[kristianmandrup]

Great! I'm so happy!!! Strange that it required various conditionals for different Ruby versions :O Good job :)

[kristianmandrup]

Oh, I think you should update the README as well if you haven't already :)

[apneadiving]

You already did it if I recall well :)

Sent from my iPhone

On 6 août 2012, at 23:12, Kristian Mandrup notifications@github.com wrote:

Oh, I think you should update the README as well if you haven't already :)

— Reply to this email directly or view it on GitHub.

[tetherit]

Geocoder stores a :coordinates field as [lng, lat] instead of [lat, lng]. Here is an example of what I'm doing in my model:

class Scope
  include Mongoid::Document
  include Geocoder::Model::Mongoid
  after_validation :geocode, :if => :address_changed?
  geocoded_by :address
  acts_as_gmappable :position => :coordinates, :process_geocoding => false

  field :name
  field :address
  field :coordinates, type: Array
end

The problem here is when I do @location.to_gmaps4rails the longitude and latitude in the json are the wrong way round.

Any way to tell :position => :coordinates that the coordinates are stored in Geocoder style of [lng, lat]?

[kristianmandrup]

I would add an :order option that by default is [:lat, :lng]. Make the changes necessary, a spec for it and then make a pull request.

[kristianmandrup]

I fixed this in my latest pull request ;) now you can specify :pos_order => [:lng, :lat] also see my README. Cheers!

[tetherit]

Thank you, that's awesome :)