I'd agree with what David said. But I'd also point out that if you 
really want to test those methods independently, you have the option of 
using instance_variable_set and instance_variable_get - so you can set 
up the object directly into a known state (even if the public API does 
not allow you to do this), then run a method, then check the state 
(ditto).

Alternatively, you can refactor into a more functional style. You got 
part way with this:

  def clean_addresses(raw_addresses) # <----- passed explicitly
    raw_addresses.each do |this_address|
      @addresses << GisTools.normalize_address(this_address)
    end
  end

However you could take this further to:

  def clean_addresses
    @addresses = do_clean_addresses(@raw_addresses)
  end

  def do_clean_addresses(raw_addresses)
    addresses = []
    raw_addresses.each do |this_address|
      addresses << GisTools.normalize_address(this_address)
    end
    addresses
  end

Now do_clean_addresses is very easy to test, and you can also 
drastically simplify it using #map or #collect:

  def do_clean_addresses(raw_addresses)
    raw_addresses.map do |this_address|
      GisTools.normalize_address(this_address)
    end
  end

At which point, you realise that do_clean_addresses probably doesn't 
need a unit test, because it's just applying GisTools.normalize_address 
to a collection.

Now, you've probably spotted the subtle API change I've implemented 
here. Each time clean_addresses is called, @addresses is reset. In your 
original API you just appended to @addresses, so that means if you 
called clean_addresses more than once, @addresses would grow and grow.

It's up to you to decide whether this is what you actually want. If you 
do, you can revert to your original behaviour like this:

  def clean_addresses
    @addresses.concat(do_clean_addresses(@raw_addresses))
  end

Maybe that's irrelevant, since clean_addresses is a private method, and 
only called once at initialization time.

But in that case, it seems pointless to set @addresses and 
@raw_addresses to [] in your initialize method, before appending to them 
later, when load_addresses and clean_addresses could be responsible for 
initializing the instance variables too. Indeed, why not just have:

  def initialize(raw_addresses)
    @raw_addresses = raw_addresses
    clean_addresses(@raw_addresses)
  end

where clean_addresses is responsible for initializing @addresses as 
described above. (Another subtle change: @raw_addresses contains the 
same array as was passed in. Usually this is considered fine, but to get 
the exact same semantics as you had before, you can do @raw_addresses = 
raw_addresses.dup)

Another option to consider is memoisation. If clean_addresses is an 
expensive computation, then you can leave it until when it is needed.

  def initialize(raw_addresses = [])
    @raw_addresses = raw_addresses
    @addresses = nil
  end

  def addresses
    @addresses ||= clean_addresses(@raw_addresses)
  end

So 'addresses' looks like a read accessor for the @addresses variable, 
but it also does the cleaning "on demand" when first called.

Hope this gives you a few ideas.

Regards,

Brian.
-- 
Posted via http://www.ruby-forum.com/.