# Unit Testing and Instance Vars

**URL:** <https://www.ruby-forum.com/t/unit-testing-and-instance-vars/172684>\
**Category:** Ruby\
**Created:** [July 25, 2009, 1:27pm UTC](https://www.ruby-forum.com/t/unit-testing-and-instance-vars/172684 "2009-07-25T13:27:33Z")\
**Posts on this page:** 5\
**Page:** 1

<div class="post-metadata">

**Author:** ![S-gw](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/ed655f/32.png) [@S-gw](https://www.ruby-forum.com/u/S-gw)\
**Post date:** [July 25, 2009, 1:27pm UTC](https://www.ruby-forum.com/t/unit-testing-and-instance-vars/172684/1 "2009-07-25T13:27:33Z")

</div>

I find myself often wondering about some design choices that result from  
code which looks similar to the contrived example below.

class Places

attr\_reader :addresses

def initialize(some\_source)  
@addresses = []  
@raw\_addresses = []

```
load_addresses(some_source)

```

end

def load\_addresses(some\_source)  
some\_source.each do |this\_source|  
@raw\_addresses \<\< this\_source.address  
end  
clean\_addresses  
end

private

def clean\_addresses  
@raw\_addresses.each do |this\_address|  
@addresses \<\< GisTools.normalize\_address(this\_address)  
end  
end  
end

Testing for the final results of @addresses is easy as the instance var  
is exposed through the attr\_reader, but there’s two problems:

A) Testing clean\_addresses is dependent on running load\_addresses first  
in order to acquire data.

B) Seeing the intermediate results of @raw\_addresses is not possible for  
testing load\_addresses independently because @raw\_addresses is not  
exposed.

To fix (A), we could insist on passing parameters. So, we would have:

def load\_addresses(some\_source)  
some\_source.each do |this\_source|  
@raw\_addresses \<\< this\_source.address  
end  
clean\_addresses(@raw\_addresses) # \<----- passed explicitly  
end

private

def clean\_addresses(raw\_addresses) # \<----- passed explicitly  
raw\_addresses.each do |this\_address|  
@addresses \<\< GisTools.normalize\_address(this\_address)  
end  
end

That would allow clean\_addresses to be tested independently and be fed  
data directly by the test which could be a good thing. Regardless of  
that advantage, I’ve often wondered if requiring the passing of instance  
vars as parameters really is a ‘higher standard’ or not. I see a lot of  
code both ways. I tend to believe it’s better to be explicit, but admit  
it feels simpler in many cases to just use the hard-coded instance var.  
I find myself flip flopping (always a bad thing).

Either way, I still can’t see the results of @raw\_addresses. I find  
myself having to expose :raw\_addresses so that tests can see the  
results, but there really is no need for :raw\_addresses to be exposed,  
and it shouldn’t be a part of the interface.

Questions:

1. Any consensus on the parameters issue (_always_ pass params, or  
_usually_)?
2. What’s the ‘best practice’ approach to seeing the results of  
@raw\_addresses?

Many thanks for the discussion.

– gw

---

<div class="post-metadata">

**Author:** ![S-gw](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/ed655f/32.png) [@S-gw](https://www.ruby-forum.com/u/S-gw)\
**Post date:** [July 25, 2009, 1:55pm UTC](https://www.ruby-forum.com/t/unit-testing-and-instance-vars/172684/2 "2009-07-25T13:55:26Z")

</div>

Hi –

On Sat, 25 Jul 2009, Greg W. wrote:

> private
> 
> some\_source.each do |this\_source|  
> end  
> Either way, I still can’t see the results of @raw\_addresses. I find  
> myself having to expose :raw\_addresses so that tests can see the  
> results, but there really is no need for :raw\_addresses to be exposed,  
> and it shouldn’t be a part of the interface.
> 
> Questions:
> 
> 1. Any consensus on the parameters issue (_always_ pass params, or  
> _usually_)?
> 2. What’s the ‘best practice’ approach to seeing the results of  
> @raw\_addresses?

The key word, I think, is “interface”, which in your example consists  
of initialize and load\_addresses. A year or so ago someone asked on  
either this list or the Rails list about testing private methods, and  
I said, yes, test all your private methods. David C. chimed in  
with the observation that testing private methods is not generally  
considered best practice; rather, the private methods are there to  
help the class do what it has to do so that its public interface will  
act correctly.

So I’m not sure you’d want to test clean\_addresses directly. And the  
fact that @raw\_addresses isn’t exposed is fine. Exposing instance  
variables is just an implementation technique for attributes anyway.  
You should be able to move instance variables around in various ways  
without changing your tests – whereas “attributes” are methods and  
therefore part of the public interface.

What you really need to verify, in the large, is that calling  
initialize with known input produces the expected array of addresses.  
You could get more granular and unit-test load\_addresses, since it’s  
also part of the public API. After that, you sink below the horizon of  
implementation detail.

At least, that’s my take on it.

David

---

<div class="post-metadata">

**Author:** ![S-gw](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/ed655f/32.png) [@S-gw](https://www.ruby-forum.com/u/S-gw)\
**Post date:** [July 25, 2009, 7:54pm UTC](https://www.ruby-forum.com/t/unit-testing-and-instance-vars/172684/3 "2009-07-25T19:54:42Z")

</div>

David A. Black wrote:

> On Sat, 25 Jul 2009, Greg W. wrote:

> > private
> > 
> > some\_source.each do |this\_source|  
> > end  
> > Either way, I still can’t see the results of @raw\_addresses. I find  
> > myself having to expose :raw\_addresses so that tests can see the  
> > results, but there really is no need for :raw\_addresses to be exposed,  
> > and it shouldn’t be a part of the interface.
> > 
> > Questions:
> > 
> > 1. Any consensus on the parameters issue (_always_ pass params, or  
> > _usually_)?
> > 2. What’s the ‘best practice’ approach to seeing the results of  
> > @raw\_addresses?
> 
> The key word, I think, is “interface”, which in your example consists  
> of initialize and load\_addresses. A year or so ago someone asked on  
> either this list or the Rails list about testing private methods, and  
> I said, yes, test all your private methods. David C. chimed in  
> with the observation that testing private methods is not generally  
> considered best practice; rather, the private methods are there to  
> help the class do what it has to do so that its public interface will  
> act correctly.

Ah, yes. I recall reading some debates about that. When thinking about  
this, I was indeed going after the test all methods stance, but I  
actually like the idea of testing the interface. If changing an  
implementation doesn’t break the application, it could be argued that it  
shouldn’t break the tests either.

– gw

---

<div class="post-metadata">

**Author:** ![S-gw](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/ed655f/32.png) [@S-gw](https://www.ruby-forum.com/u/S-gw)\
**Post date:** [July 25, 2009, 2:54pm UTC](https://www.ruby-forum.com/t/unit-testing-and-instance-vars/172684/4 "2009-07-25T14:54:33Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![S-gw](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/ed655f/32.png) [@S-gw](https://www.ruby-forum.com/u/S-gw)\
**Post date:** [July 25, 2009, 8:06pm UTC](https://www.ruby-forum.com/t/unit-testing-and-instance-vars/172684/5 "2009-07-25T20:06:35Z")

</div>

Brian C. wrote:

> 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

Mmm, wasn’t aware of those. Yes, that could be handy.

> Alternatively, you can refactor…

Yeah, this was a quickly contrived example to show the testing  
questions. I tend to like lazy loading (and therefore memoization)  
approaches to things where its possible, and that would have driven me  
to end up down a path similar to what you did if this were real code.

– gw