Sorry for such a nub question but
How would I move the majority of this logic to the model?
def save_sub
page = Page.find(params[:id])
subpage = page.children.create(params[:form])
if subpage.save
flash[:notice] = "Saved Changes"
redirect_to :action => "view_page", :id => subpage
else
render :action => "add_sub"
end
end
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
Why would you want to? The controller is the place for this logic surely? On 5/14/07, Vinnie <vincent-franco-Wuw85uim5zDR7s880joybQ@public.gmane.org> wrote:> > > Sorry for such a nub question but > How would I move the majority of this logic to the model? > > def save_sub > page = Page.find(params[:id]) > subpage = page.children.create(params[:form]) > > if subpage.save > flash[:notice] = "Saved Changes" > redirect_to :action => "view_page", :id => subpage > else > render :action => "add_sub" > end > end > > > >-- http://www.web-buddha.co.uk --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Aye, that''s perfectly fine Controller code. Short and to the point. Jason On 5/14/07, Dave Goodchild <buddhamagnet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> > Why would you want to? The controller is the place for this logic surely? > > On 5/14/07, Vinnie <vincent-franco-Wuw85uim5zDR7s880joybQ@public.gmane.org > wrote: > > > > > > Sorry for such a nub question but > > How would I move the majority of this logic to the model? > > > > def save_sub > > page = Page.find(params[:id]) > > subpage = page.children.create(params[:form]) > > > > if subpage.save > > flash[:notice] = "Saved Changes" > > redirect_to :action => "view_page", :id => subpage > > else > > render :action => "add_sub" > > end > > end > > > > > > > > > > > -- > http://www.web-buddha.co.uk > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
I would suggest perhaps an add_child method on instances of Page. This way if the implementation of how Pages and child Pages are related in your model changes, your controllers are shielded from the change. The Law of Demeter (http://en.wikipedia.org/wiki/Law_of_Demeter) is a good one to try and follow wherever realistic. - Gabriel On 5/14/07, Jason Roelofs <jameskilton-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> Aye, that''s perfectly fine Controller code. Short and to the point. > > Jason > > > On 5/14/07, Dave Goodchild < buddhamagnet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > Why would you want to? The controller is the place for this logic surely? > > > > > > On 5/14/07, Vinnie < vincent-franco-Wuw85uim5zDR7s880joybQ@public.gmane.org > wrote: > > > > > > Sorry for such a nub question but > > > How would I move the majority of this logic to the model? > > > > > > def save_sub > > > page = Page.find(params[:id]) > > > subpage = page.children.create(params[:form]) > > > > > > if subpage.save > > > flash[:notice] = "Saved Changes" > > > redirect_to :action => "view_page", :id => subpage > > > else > > > render :action => "add_sub" > > > end > > > end > > > > > > > > > > > > > > > > > > > > -- > > http://www.web-buddha.co.uk > > > > > > > > > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Your subpage.save call is unnecessary, create saves your object for you. You can just check if the created object is a new_record? (subpage.new_record?) or not to determine whether or not the save was successful. And like was already mentioned the only other thing you could do to improve this is to create a method on your page object that hides the relationship from page to children but I don''t think it''s worth the extra abstraction unless you feel like that part of your architecture is going to change. Anyway, your call, what you have now is fine. -Michael http://javathehutt.blogspot.com On May 14, 2007, at 9:25 AM, Vinnie wrote:> > Sorry for such a nub question but > How would I move the majority of this logic to the model? > > def save_sub > page = Page.find(params[:id]) > subpage = page.children.create(params[:form]) > > if subpage.save > flash[:notice] = "Saved Changes" > redirect_to :action => "view_page", :id => subpage > else > render :action => "add_sub" > end > end > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Agreed with the others. Most of this code is about what gets presented in the browser, none of that stuff should be in the model in any way. If you want dry things up, one pattern I sometimes use is: before_filter :load_object private def load_object @object = Object.find(params[:id]) if params[:id] end But that may not be worth it depending on the rest of your controller. On May 14, 10:25 am, Vinnie <vincent-fra...-Wuw85uim5zDR7s880joybQ@public.gmane.org> wrote:> Sorry for such a nub question but > How would I move the majority of this logic to the model? > > def save_sub > page = Page.find(params[:id]) > subpage = page.children.create(params[:form]) > > if subpage.save > flash[:notice] = "Saved Changes" > redirect_to :action => "view_page", :id => subpage > else > render :action => "add_sub" > end > end--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 5/14/07, Michael Kovacs <kovacs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> I don''t think it''s worth the extra abstraction unless you feel like > that part of your architecture is going to changeIt''s a fine line between under-abstraction and over-abstraction, but I''ve found the parts you think are most unlikely to change are the ones that end up hurting the most to change unless you''ve taken reasonable preparatory measures :) - Gabriel --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---