views:

342

answers:

1

My model looks like this:

class Item < ActiveRecord::Base
  has_many :locations
  validate :validate_item_location

  def item_location
    locations.address+','+locations.city+','+locations.country
  end

  def item_location=(str)
    geo = Geokit::Geocoders::MultiGeocoder.geocode(str)
    if geo.success
      locations.build( :lat => geo.lat, :lng => geo.lng)
    end
  end

  def validate_item_location
    geo = Geokit::Geocoders::MultiGeocoder.geocode( item_location )
    errors.add_to_base("Location is invalid") unless geo.success
  end
end

My questions 1. How to correctly write a getter method item_location defined? 2. How can I validate item_location field. I created validate_item_location method but don't know how to get item_location variable inside when I POST data through my form. 3. Is my setter method ok?

THX!

+2  A: 

1) An Item can have many locations? It seems (to me) that it should have only one, so change hasy_many to has_one. Unless you really wanted to have multiple locations, then you need to change item_location to choose one location from the list you have.

2 & 3) If you POST your data through a form, the item_location gets set by the item_location= method. Which should (somehow) store the item information. In your case it stores the coordinates returned from the geo variable. You should raise some error, when geo.success is false to notify the user that the value was not stored. If you specifically want to validate the value send to the setter, then you need to store it in the class: @saved_location = str and use @saved_location to validate, instead of item_location.

1 & 3) In general it is practice that, a setter and a getter use the same data(structure). In your case you store the coordinates of the position in your setter, but give back the address, city and country. Thus the setter and the getter seem to be incompatible.

Hope these remarks help!

Veger