I have some questions regarding how to avoid having to duplicate code
in the controller between certain actions such as "new",
"create",
"edit", "update", etc.
Currently, my "new" and "update" actions both load a couple
collections of reference data used to populate drop-down/select
controls in the view. To avoid duplicating the same code in both
actions, I am defining a before_filter:
[code]
class MyController
before_filter :find_states, :only => [:new, :edit]
before_filter :find_categories, :only => [:new, :edit]
... # action definitions
private
def find_states
@states = State.find(:all, :order => "name")
end
def find_categories
@categories = Category.find(:all, :order => "name")
end
end
[/code]
So this is all well and good, the new and edit actions are just fine.
However, my issue is really with the "create" and "update"
actions
which are responsible for actually updating the DB. In the case where
the save or update operations fail, these methods will try to render
the "new" or "edit" views. In this case, I need to make sure
the
reference data that is required is populated.
I see several solutions to this problem, but none of them particularly
strike me as great solutions, or seem particularly DRY. So I guess I''m
just looking for something a little better/cleaner/DRYer, however you
want to put it.
Anyhow, my current ideas for solutions, none of which I really like,
are as follows:
1. In the "create" and "update" actions, call the filter
methods
prior to rendering the new/edit views (don''t like this, since if
additional reference data is added later, I will have to add/remove
more method calls)
2. Add additional helper methods that set all of the reference data
for a particular action (i.e. find_ref_data_for_new) (this is better
than option 1, but would still potentially have to modify code in
multiple locations)
3. Add "create" and "update" to the list of actions
included in the
before_filter (this would work, but it just doesn''t make sense, since
"create" and "update" are really DB related actions, and
only display
data when some DB operation, or validation, fails)
Any help or suggestions would be greatly appreciated.
- Justin
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
ck.>> Anyhow, my current ideas for solutions, none of which I really like, > are as follows: > 1. In the "create" and "update" actions, call the filter methods > prior to rendering the new/edit views (don''t like this, since if > additional reference data is added later, I will have to add/remove > more method calls) > > 2. Add additional helper methods that set all of the reference data > for a particular action (i.e. find_ref_data_for_new) (this is better > than option 1, but would still potentially have to modify code in > multiple locations) > > 3. Add "create" and "update" to the list of actions included in the > before_filter (this would work, but it just doesn''t make sense, since > "create" and "update" are really DB related actions, and only display > data when some DB operation, or validation, fails) >#1: Dislike this for your very reasons. #3: Dislike this because I like the idea of parsimony in the execution of the application. In the create and update actions, you don''t always need that reference data, and to fetch that data every time, is unnecessary. #2: Seems to be the best compromise between too much coding, and too much execution. -- 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 -~----------~----~----~----~------~----~------~--~---
Hey Justin,
I''ll recommend that every data used to load drop down lists on the
views should be loaded on the views, that avoids repetition on the
controllers. For example a list of post categories you can load it
like this:
<%= collection_select(:post, :post_category_id,
PostCategory.find(:all), :id, :name, {}) %>
Hope it helps,
Elías
On Nov 8, 12:30 pm, Justin Holzer
<jshol...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:> I have some questions regarding how to avoid having to duplicate code
> in the controller between certain actions such as "new",
"create",
> "edit", "update", etc.
>
> Currently, my "new" and "update" actions both load a
couple
> collections of reference data used to populate drop-down/select
> controls in the view. To avoid duplicating the same code in both
> actions, I am defining a before_filter:
>
> [code]
> class MyController
> before_filter :find_states, :only => [:new, :edit]
> before_filter :find_categories, :only => [:new, :edit]
>
> ... # action definitions
>
> private
>
> def find_states
> @states = State.find(:all, :order => "name")
> end
>
> def find_categories
> @categories = Category.find(:all, :order => "name")
> end
> end
> [/code]
>
> So this is all well and good, the new and edit actions are just fine.
>
> However, my issue is really with the "create" and
"update" actions
> which are responsible for actually updating the DB. In the case where
> the save or update operations fail, these methods will try to render
> the "new" or "edit" views. In this case, I need to make
sure the
> reference data that is required is populated.
>
> I see several solutions to this problem, but none of them particularly
> strike me as great solutions, or seem particularly DRY. So I guess
I''m
> just looking for something a little better/cleaner/DRYer, however you
> want to put it.
>
> Anyhow, my current ideas for solutions, none of which I really like,
> are as follows:
> 1. In the "create" and "update" actions, call the
filter methods
> prior to rendering the new/edit views (don''t like this, since if
> additional reference data is added later, I will have to add/remove
> more method calls)
>
> 2. Add additional helper methods that set all of the reference data
> for a particular action (i.e. find_ref_data_for_new) (this is better
> than option 1, but would still potentially have to modify code in
> multiple locations)
>
> 3. Add "create" and "update" to the list of actions
included in the
> before_filter (this would work, but it just doesn''t make sense,
since
> "create" and "update" are really DB related actions,
and only display
> data when some DB operation, or validation, fails)
>
> Any help or suggestions would be greatly appreciated.
>
> - Justin
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
Elias Orozco wrote:> Hey Justin, > > I''ll recommend that every data used to load drop down lists on the > views should be loaded on the views, that avoids repetition on the > controllers. For example a list of post categories you can load it > like this: > > <%= collection_select(:post, :post_category_id, > PostCategory.find(:all), :id, :name, {}) %> > > Hope it helps, > > El�asThat''s a good suggestion, Elias. I''ll add that, if the drop-down list is going to be in multiple views, I''d put it in a helper method. -Nick -- 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@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Nick, That''s a great suggestion. I would actually prefer using a helper method over putting the call to ModelObject#find in the view itself. Something about calling find directly from the view just doesn''t feel right to me. The only reason I''m not 100% opposed to it is because a call to find is usually going to be a 1-liner. Even so, it still just feels like it doesn''t belong directly in the view itself. I am new to Rails, but based on my perception of MVC, fetching data from the model is a task that is meant to be done by the controller, and the view should simply be handed that data by the controller. However, looking through the Rails documentation itself, I saw the same method mentioned by Elias (calling find directly in the view) in examples within the ActionView::FormOptionsHelper class, so I guess that is a Rails convention? Or was it just done for example purposes? If you start having to order your reference data, and do other operations where it''s not as simple as a call to find(:all), then I think that could really muddy up the view code. A helper method would certainly be preferable, perhaps something like category_select(...), which handles grabbing the data from the model. At least you''re not shoving the find call directly in to the view, and you still gain the benefit of not having to call the same find() method from all over the controller. So far, I have been absolutely blown away by almost all the features that Rails has to offer, but it almost seems like the designers of the framework overlooked, or didn''t feel it was important, to add a simple, and DRY, way to fetch reference data from the controller. I am quite surprised by this, as it is a very common task to fetch reference data for the purpose of populating controls such as select/ drop-down boxes. I was also surprised at the lack of message board and blog posts about this subject. Perhaps, as I mentioned, the method that Elias made reference to is the Rails convention, and I am just missing something :) Thanks to everyone for all the great suggestions. Keep em coming. Regards, Justin On Nov 11, 11:15 am, Nick Hoffman <rails-mailing-l...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org> wrote:> Elias Orozco wrote: > > Hey Justin, > > > I''ll recommend that every data used to load drop down lists on the > > views should be loaded on the views, that avoids repetition on the > > controllers. For example a list of post categories you can load it > > like this: > > > <%= collection_select(:post, :post_category_id, > > PostCategory.find(:all), :id, :name, {}) %> > > > Hope it helps, > > > El as > > That''s a good suggestion, Elias. I''ll add that, if the drop-down list is > going to be in multiple views, I''d put it in a helper method. > -Nick > -- > 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 -~----------~----~----~----~------~----~------~--~---
G''day Justin.> That''s a great suggestion. I would actually prefer using a helper > method over putting the call to ModelObject#find in the view itself. > Something about calling find directly from the view just doesn''t feel > right to me. The only reason I''m not 100% opposed to it is because a > call to find is usually going to be a 1-liner.It might be a one-liner right now, but once you add error handling, it''s longer. And what''s error handling?...It''s logic, which doesn''t belong in views.> I am new to Rails, but based on my perception of MVC, fetching data > from the model is a task that is meant to be done by the controller, > and the view should simply be handed that data by the controller.Yup! Absolutely.> However, looking through the Rails documentation itself, I saw the > same method mentioned by Elias (calling find directly in the view) in > examples within the ActionView::FormOptionsHelper class, so I guess > that is a Rails convention? Or was it just done for example purposes?There could be all sorts of reasons why the example''s writer put the call to #find in the view. Regardless, "The Rails Way" is to not put logic in views.> So far, I have been absolutely blown away by almost all the features > that Rails has to offer, but it almost seems like the designers of the > framework overlooked, or didn''t feel it was important, to add a > simple, and DRY, way to fetch reference data from the controller.I think you might be confusing the act of "fetching" reference data with the act of formatting and/or displaying it. Controllers share data (IE: variables) with views via instance variables, or passing variables to the ":locals" option of #render. That''s pretty simple. If multiple controller actions need to share the same data, simply write a protected or private controller method, or add a method to the model in question, that all of these controller actions can call.> I am quite surprised by this, as it is a very common task to fetch > reference data for the purpose of populating controls such as select/ > drop-down boxes. I was also surprised at the lack of message board and > blog posts about this subject. Perhaps, as I mentioned, the method > that Elias made reference to is the Rails convention, and I am just > missing something :)As you know, views shouldn''t contain any sort of logic. If you''re thinking about calling #find within a view, what will you do with the data that''s returned? Most of the time, you''ll need to check if any records were returned. After that, you''ll iterate through the Array, and display some data. That''s "business logic", which doesn''t belong in views. But it does belong in helpers! -Nick -- 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 -~----------~----~----~----~------~----~------~--~---
Nick,
Thanks again for the reply.
I think I will probably go back to using either protected controller
methods or helper methods to fetch the reference data. I agree with
you 100% in that this type of logic simply has no place in the view.
As for my statement regarding the framework lacking a way to easily,
and DRYly, fetch reference data... I am just surprised that there is
not any kind of shortcut/helper for loading reference data now.
Perhaps a function built in to ActiveController such as
"load_ref_data" which could optionally take an argument that would
specify the name of the action to load data for (would default to
current action), and could potentially use some type of naming
convention to call a method that gets all of the reference data for
the action. I''m sure there are more clever ways to do this, and this
is just off the top of my head.
So you might have something in you controller such as...
def new
load_ref_data
...
end
def create
...
if @myObject.save
...
else
load_ref_data :new
render :action => :new
end
protected
def ref_data_new
@categories = Category.find(...)
@states = State.find(...)
...
end
You might even add an option to the render method itself, such as
"render :action => :new, :include_ref_data => true".
I will probably end up rolling my own system, but I am surprised with
all the things that they thought of for Rails, that they didn''t think
of something like this.
- Justin
On Nov 16, 11:13 am, Nick Hoffman
<rails-mailing-l...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org>
wrote:> G''day Justin.
>
> > That''s a great suggestion. I would actually prefer using a
helper
> > method over putting the call to ModelObject#find in the view itself.
> > Something about calling find directly from the view just
doesn''t feel
> > right to me. The only reason I''m not 100% opposed to it is
because a
> > call to find is usually going to be a 1-liner.
>
> It might be a one-liner right now, but once you add error handling,
it''s
> longer. And what''s error handling?...It''s logic, which
doesn''t belong in
> views.
>
> > I am new to Rails, but based on my perception of MVC, fetching data
> > from the model is a task that is meant to be done by the controller,
> > and the view should simply be handed that data by the controller.
>
> Yup! Absolutely.
>
> > However, looking through the Rails documentation itself, I saw the
> > same method mentioned by Elias (calling find directly in the view) in
> > examples within the ActionView::FormOptionsHelper class, so I guess
> > that is a Rails convention? Or was it just done for example purposes?
>
> There could be all sorts of reasons why the example''s writer put
the
> call to #find in the view. Regardless, "The Rails Way" is to not
put
> logic in views.
>
> > So far, I have been absolutely blown away by almost all the features
> > that Rails has to offer, but it almost seems like the designers of the
> > framework overlooked, or didn''t feel it was important, to add
a
> > simple, and DRY, way to fetch reference data from the controller.
>
> I think you might be confusing the act of "fetching" reference
data with
> the act of formatting and/or displaying it. Controllers share data (IE:
> variables) with views via instance variables, or passing variables to
> the ":locals" option of #render. That''s pretty simple.
If multiple
> controller actions need to share the same data, simply write a protected
> or private controller method, or add a method to the model in question,
> that all of these controller actions can call.
>
> > I am quite surprised by this, as it is a very common task to fetch
> > reference data for the purpose of populating controls such as select/
> > drop-down boxes. I was also surprised at the lack of message board and
> > blog posts about this subject. Perhaps, as I mentioned, the method
> > that Elias made reference to is the Rails convention, and I am just
> > missing something :)
>
> As you know, views shouldn''t contain any sort of logic. If
you''re
> thinking about calling #find within a view, what will you do with the
> data that''s returned? Most of the time, you''ll need to
check if any
> records were returned. After that, you''ll iterate through the
Array, and
> display some data.
>
> That''s "business logic", which doesn''t belong
in views. But it does
> belong in helpers!
> -Nick
> --
> 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
-~----------~----~----~----~------~----~------~--~---