Roy Pardee
2008-Feb-21 18:38 UTC
noob: where to put the find(:all) to use with collection_select() in multiple places?
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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Stefan Lang
2008-Feb-21 20:49 UTC
Re: noob: where to put the find(:all) to use with collection_select() in multiple places?
2008/2/21, Roy Pardee <rpardee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>: 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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Roy Pardee
2008-Feb-21 22:06 UTC
Re: noob: where to put the find(:all) to use with collection_select() in multiple places?
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...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> 2008/2/21, Roy Pardee <rpar...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>: > > 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--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Mark Bush
2008-Feb-22 11:08 UTC
Re: noob: where to put the find(:all) to use with collection
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/. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Roy Pardee
2008-Feb-22 17:23 UTC
Re: noob: where to put the find(:all) to use with collection
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...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org> wrote:> 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 viahttp://www.ruby-forum.com/.--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Robert Walker
2008-Feb-22 18:32 UTC
Re: noob: where to put the find(:all) to use with collection
Doesn''t Rails 2.0 already do SQL caching? On Feb 22, 12:23 pm, Roy Pardee <rpar...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> 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...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org> > wrote: > > > 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 viahttp://www.ruby-forum.com/.--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---