# Best way to thin out controller

**URL:** <https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143>\
**Category:** Rails\
**Created:** [May 14, 2007, 6:27pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143 "2007-05-14T18:27:29Z")\
**Posts on this page:** 7\
**Page:** 1

<div class="post-metadata">

**Author:** ![vinnie](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/v/71e660/32.png) [@vinnie](https://www.ruby-forum.com/u/vinnie)\
**Post date:** [May 14, 2007, 6:27pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143/1 "2007-05-14T18:27:29Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![vinnie](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/v/71e660/32.png) [@vinnie](https://www.ruby-forum.com/u/vinnie)\
**Post date:** [May 14, 2007, 7:03pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143/2 "2007-05-14T19:03:34Z")

</div>

Aye, that’s perfectly fine Controller code. Short and to the point.

Jason

---

<div class="post-metadata">

**Author:** ![vinnie](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/v/71e660/32.png) [@vinnie](https://www.ruby-forum.com/u/vinnie)\
**Post date:** [May 14, 2007, 6:33pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143/3 "2007-05-14T18:33:30Z")

</div>

Why would you want to? The controller is the place for this logic  
surely?

On 5/14/07, Vinnie [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid) wrote:

> ```
> flash[:notice] = "Saved Changes"
> redirect_to :action => "view_page", :id => subpage
> else
> render :action => "add_sub"
> end
> 
> ```
> 
> end
> 
> >

–  
[http://www.web-buddha.co.uk](http://www.web-buddha.co.uk)

---

<div class="post-metadata">

**Author:** ![vinnie](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/v/71e660/32.png) [@vinnie](https://www.ruby-forum.com/u/vinnie)\
**Post date:** [May 14, 2007, 7:39pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143/4 "2007-05-14T19:39:39Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![vinnie](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/v/71e660/32.png) [@vinnie](https://www.ruby-forum.com/u/vinnie)\
**Post date:** [May 14, 2007, 7:14pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143/5 "2007-05-14T19:14:36Z")

</div>

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 ([Law of Demeter - Wikipedia](http://en.wikipedia.org/wiki/Law_of_Demeter)) is a good  
one to try and follow wherever realistic.

- Gabriel

---

<div class="post-metadata">

**Author:** ![vinnie](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/v/71e660/32.png) [@vinnie](https://www.ruby-forum.com/u/vinnie)\
**Post date:** [May 14, 2007, 7:39pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143/6 "2007-05-14T19:39:39Z")

</div>

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](http://javathehutt.blogspot.com)

---

<div class="post-metadata">

**Author:** ![vinnie](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/v/71e660/32.png) [@vinnie](https://www.ruby-forum.com/u/vinnie)\
**Post date:** [May 14, 2007, 7:57pm UTC](https://www.ruby-forum.com/t/best-way-to-thin-out-controller/104143/7 "2007-05-14T19:57:42Z")

</div>

On 5/14/07, Michael K. [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid) wrote:

> I don’t think it’s worth the extra abstraction unless you feel like  
> that part of your architecture is going to change

It’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