noob: where to put the find(:all) to use with collection_select() in multiple places?

[Roy Pardee]

Hey All,

I'm just starting out w/rails & have a toy app w/just 2 types of
things right now: People and Organizations. Person :belongs_to
organization and Organization :has_many people.

I'd like to provide a select control showing the available
organizations on the people/new and people/edit actions (or whatever
they're called) and so I'm using collection_select to do that. My
question is where to put the call to Organization.find(:all) to grab
out the data I need to populate the select control. It seems like I
should be able to do that just once & stash the results in a class var
for use anyplace in the people controller class.

But when I go to put that list in a class variable in my (scaffold-
generated) people_controller.rb file like so:

class PeopleController < ApplicationController

@@organizations = Organization.find(:all)

def new
# blah blah blah
end

# etc...

end

And then try to access this from views/people/new.html.erb w/this
line:

<%= collection_select(:person, :organization_id,
@@organizations, :id, :name) %>

I get a NameError on that call to collection_select() w/msg
"uninitialized class variable @@organizations in
ActionView::Base::CompiledTemplates"

So... where can I stash the results of that .find(:all) call so that
the same data is available for both actions?

Thanks!

-Roy

[Stefan Lang]

2008/2/21, Roy Pardee <rpa...@gmail.com>:

Hi,

> <%= collection_select(:person, :organization_id,
> @@organizations, :id, :name) %>

Even if this would work, I'd strongly advise against
it since the view has more knowledge of your controller
than it needs to and turns your nice MVC app into
a tangled mess. Just use a normal instance variable
here (@organizations). That gives the controller a chance
to manipulate the organizations list (e.g. certain users
may only create people for a subset of all available
organizations, etc.) without requiring changes in the view.

Regarding the controller: Isn't it possible that the
list of organizations changes? Your current code doesn't
account for that. In this case the right way is a simple

def new
@organizations = Organization.find(:all)
...

def edit
@organizations = Organization.find(:all)
...

If the organizations don't change, and you _really_
want to load the only once, just add

def new
@organizations = @@organizations
...

HTH,
Stefan

[Roy Pardee]

Thanks for the response Stefan!

You're absolutely right that organizations can change--in the fullness
of time I'm going to want to stash that list in whatever rails has for
an application-wide cache, and refresh it whenever the list changes.
(I think there are activerecord hook methods I can use for that,
aren't there?).

But for now I'm just trying to get oriented to rails. It seems
strange to me that instance vars are available in the view, but class
vars aren't, and putting multiple calls to .find(:all) seems overly
chatty w/the database, and not as DRY as could be.

Maybe what I need to do is just read up on caching in rails & see what
I can see on that...

Thanks!

-Roy

On Feb 21, 12:49 pm, "Stefan Lang" <perfectly.normal.hac...@gmail.com>
wrote:
> 2008/2/21, Roy Pardee <rpar...@gmail.com>:

[Mark Bush]

Roy Pardee wrote:
> But for now I'm just trying to get oriented to rails. It seems
> strange to me that instance vars are available in the view, but class
> vars aren't, and putting multiple calls to .find(:all) seems overly
> chatty w/the database, and not as DRY as could be.
>
> Maybe what I need to do is just read up on caching in rails & see what
> I can see on that...

It's not that this isn't DRY, it's that each call to the method must
allow for the fact that the Organization model changes. The controller
shouldn't assume that it is the only way of accessing a model which is
what you'll do by updating a controller class variable whenever the
controller changes a model. This will lead to problems down the line if
you end up with other accesses to the model. This may not seem
important for this application, but it would be a bad habit to get into
as it will definitely bite you on a larger project.

It is unlikely you are expecting a large number of organisations as that
would make your select list unwieldy. So obtaining the list from the
database should be fairly low cost. Let your database handle data
caching and let your application handle access control and logic. When
doing a find(:all) the database will do a bulk load in one call which
shouldn't be much more expensive than getting a single row for small
amounts of data (and so comparable to the cost of adding the new People
record) and if you are using a decent database, it may have cached the
results for you anyway. Also, at this stage, you don't know how
noticable caching this value yourself will be. Use profiling to decide
where to target performance efforts.

When starting out, it's probably more useful to concentrate on
functionality rather than minor details and look at how you can refactor
later once the application takes shape. As you play with more
applications and look at other's code, you'll start recognising
opportunities for factoring earlier in development.

Finally, if you really want to cache the records, put them where they
belong. In the model. All access must go through the model, so use the
model's after_* hooks to keep a cache in the model and provide access to
this cache (which will even be available from a view if you really need
it there).
--
Posted via http://www.ruby-forum.com/.

[Roy Pardee]

Ah, that makes perfect sense--thanks much for the response. I
definitely take the point that it's too early at this point to fuss
over performance. Like I say, I'm still getting oriented--and not
just to rails I guess, but to MVC in general.

I got together w/some friends last night & they helped me to write
this bit here, which seems to work (even as the list of orgs changes)
*and* satisfies my db-chattiness concern, I think, w/out breaking the
MVC separation of concerns.

class Organization < ActiveRecord::Base
has_many :people
def self.get_list
@@all_organizations ||= self.find(:all, :select => 'id, name')
end
def before_save
@@all_organizations = nil
end
end

That gives me an Organization.get_list method that I can call from my
Person views (and any view? I guess probably...).

Thanks!

-Roy

On Feb 22, 3:08 am, Mark Bush <rails-mailing-l...@andreas-s.net>
wrote:

[Robert Walker]

Doesn't Rails 2.0 already do SQL caching?