views:

317

answers:

3

I have some players and the players have a trade state. Rather than hard code trade states like "active" and "inactive" and then have to go looking for strings, I thought I'd be clever and have a separate TradeState model so that a Player has a trade_state_id (a Player can be in only one trade state at a time).

Now, it would be a convenience to be able to get all the active players by using named scopes and then by saying "Player.active". To do that, I need to get the ID of the TradeState records that matches 'active', so I came up with this in the Player class:

named_scope :active, :conditions => {:trade_state_id => TradeState.active.first.id}

This works like a charm when tested in script/console, but it does not work when I go to test. I'm using RSpec, but I suspect that isn't pertinent. When I run the most trivial test, I get the following error:

"Called id for nil, which would mistakenly be 4"

As far as I can tell, the testing framework is loading and parsing the models in alphabetical order. The framework parses the named_scope call in the Player model and dutifully goes to look up the id for the first TradeState record that is active. However, that model hasn't been processed yet and isn't ready, hence the error about getting the id of nil.

At first I thought it was because there may not have been any records in the trade_states table, so I create and save the trade_states that I needed in the before(:each) block, but that didn't work. So then I made some fixtures and tried loading them, but that didn't work.

Does this seem plausible? Are there other explanations? How about work arounds? I could try mocking the TradeState object and I'll give that a go.

Thanks so very much for your time.

A: 

This piece of code you have mentioned gets executed when Player is "loaded". So even if you create the setup data in before(:each) block, this model might have loaded even before that. To fix the problem you can try some thing like:

named_scope :active, lambda { {:conditions => {:trade_state_id => TradeState.active.first.id} } }

This ensures that the TradeState.active is called whenever you call the named_scope.

But what are you achieving by calling TradeState.active.first. Instead you can do so by this nice way

:conditions => {:trade_state_id => TradeState::ACTIVE}

with the following code on your TradeState model:

class TradeState < ActiveRecord::Base
  def self.const_missing(sym)
    const_set(sym, find_by_name(sym.to_s))
  end
end
Selva
I like the solution to use the constant and setting it dynamically. However, when running tests, the same problem remained; namely although there were records in the trade_states table, instantiating the Player model before the TradeState model always yielded a nil reference.
Peter Degen-Portnoy
Can you post your rspec code, to get more clear picture of wats going on..
Selva
A: 

This is actually a potentially "expected" outcome. If you have no active players, then

TradeState.active

is empty, and thus

TradeState.active.first

is nil,

and thus

TradeState.active.first.id

calls Object::id on a nil object.

You might try:

TradeState.active.empty? ? 0 : TradeState.active.first[:id]

In any case, be sure you include trade_states just to make sure they are present for the join.

Steve Ross
A: 

Unfortunately, although all these suggestions were very good, nothing worked reliably when running test cases. At first I backed out all this code, but the code that I had to write was so much less attractive that I started to rethink the problem.

Because we will be looking up players by their trade state a great deal, having an index on a numerical trade state column is a clear priority. However, there is nothing that said that trade state needed to be a table, since there are only a few trade states defined and it is unlikely that there will ever be much volatility in defined trade states.

Therefore, I made TradeState a class in its own right, no longer derived from ActiveRecord, and defined the constants that I needed.

Thank you all for your thoughts and solutions.

Peter Degen-Portnoy