# Should Finds be in the controller or the model? (slimming fat controllers)

**URL:** <https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104>\
**Category:** Rails\
**Created:** [October 23, 2007, 10:27pm UTC](https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104 "2007-10-23T22:27:17Z")\
**Posts on this page:** 6\
**Page:** 1

<div class="post-metadata">

**Author:** ![bob](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/ea666f/32.png) [@bob](https://www.ruby-forum.com/u/bob)\
**Post date:** [October 23, 2007, 10:27pm UTC](https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104/1 "2007-10-23T22:27:17Z")

</div>

I’m building an app that you log into, and when you are logged in you  
should only see your data. Therefore I have a lot of instance  
variables that are dependent on a session value to pull your data into  
a view. Of course, I’ve found it’s tricky to call a session’s info in  
the model, so I’ve ended up with a bunch of fat controllers.

I know that it’s a best practice to keep your controllers as skinny as  
possible, but I’m wondering what’s the general opinion from folks who  
have had a similar functionality in their apps… do you have chunky  
controllers or do you have some tricks to slim out the controller and  
call session info in the model?

Bob.

---

<div class="post-metadata">

**Author:** ![bob](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/ea666f/32.png) [@bob](https://www.ruby-forum.com/u/bob)\
**Post date:** [October 23, 2007, 11:05pm UTC](https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104/2 "2007-10-23T23:05:20Z")

</div>

My take on this is that the find methods belong on the model’s class.  
Putting a custom find in more than one place means that if you change  
the model in the slightest you have to hunt down each caller of it.  
An example of this was when I started splitting the image data for  
uploaded pictures apart from the image meta-data such as format and  
dimensions. I used to store it base64 encoded in the same table as  
the meta-data, but found that:

Toon.find(1).avatars

returned a LOT of data from the model. Most of the time I don’t need  
it all, and while I can  
put a special form of:

Toon.find(1).avatars(:all, :select =\> “this, that, this\_other\_thing”)

in each place, I found using:

Toon.find(1).avatars.meta\_data

worked much nicer.

Thus far, in my learning of ActiveRecord, I have mostly stuck with the  
model of:

class User  
has\_many :toons  
…  
end

Class Toon  
belongs\_to :user  
…  
end

and then performed searches like:

current\_user.toons.each { |t|  
… do something with the toon …  
}

This mostly works, and keeps items returned narrowed down to that  
user. Most of the other data is either public or restricted, and  
often times part of the record is public, and part is restricted. For  
instance, anyone can look at the toons, but no one can look at the  
user who owns them other than an admin. Also, the toon’s name is  
public, but some internal recordkeeping is only shown to the owner of  
the toon or to an admin.

Recently, I’ve started adding objects which have somewhat complex  
rules on who can view or modify them. Specifically, forums. In a  
forum world, the owner of the forum (which is a toon or a guild) is  
not the only entity which can update it, so I cannot use this clear  
user.foo.bar path to get to them. In reality I could, but that would  
make one huge SQL statement, which just won’t cut it.

So, how am I getting around this?

I made custom finders for forums. For one, I have one that takes a  
Toon class as a finder:

Forums.find\_for\_toon(:all, :toon =\> Toon.find(1), :access =\> :read)

for instance. This finder takes all the standard find parameters, but  
also adds a condition to the list of ones supplied by the user, and  
returns a list of forums the Toon in question can access.

Even then, this is a huge database hit. Luckily I can do it mostly in  
one select, and weed through the output. Unluckily, there is  
potentially a lot of output.

–Michael

---

<div class="post-metadata">

**Author:** ![bob](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/ea666f/32.png) [@bob](https://www.ruby-forum.com/u/bob)\
**Post date:** [October 24, 2007, 2:48pm UTC](https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104/3 "2007-10-24T14:48:30Z")

</div>

Just pass the session data from the controller to the custom find  
method in the model.

## item\_controller.rb

@item = Item.custom\_find(session[:user])

## item.rb

def self.custom\_find(user\_id)  
self.find\_by\_user\_id(user\_id)  
end

---

<div class="post-metadata">

**Author:** ![bob](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/ea666f/32.png) [@bob](https://www.ruby-forum.com/u/bob)\
**Post date:** [October 24, 2007, 3:18pm UTC](https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104/4 "2007-10-24T15:18:56Z")

</div>

I’ve considered passing the session data to a custom find method, but it  
seemed a little bit kludgy to me when I looked at the code. It seemed to  
be  
adding unnecessary complexity. For example in the case below, if I just  
left  
the find in the controller, I would eliminate the model method and just  
have…

## item\_controller.rb

@item = Item.find\_by\_user\_id(session[:user])

That seems like a simpler solution to me, because you aren’t bouncing  
back  
and forth between the model and the controller. Of course if the find  
was  
more complicated and used in a bunch of places, centralizing it in the  
model  
would start to make more sense, but in my situation the find methods are  
only 2 or 3 lines and used in one or two different controller methods.

In Rails is it a common best practice to keep the find in the model even  
if  
it is adding some complexity in certain cases, or should I just use  
whatever  
makes sense for the situation?

---

<div class="post-metadata">

**Author:** ![bob](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/ea666f/32.png) [@bob](https://www.ruby-forum.com/u/bob)\
**Post date:** [October 26, 2007, 6:03am UTC](https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104/5 "2007-10-26T06:03:47Z")

</div>

Personally, I would put any find methods that are used more than once  
in the model. It may seem to add more complexity now but if your  
specifications ever change, you’ll be thankful for the DRYness.

---

<div class="post-metadata">

**Author:** ![bob](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/ea666f/32.png) [@bob](https://www.ruby-forum.com/u/bob)\
**Post date:** [October 24, 2007, 3:37pm UTC](https://www.ruby-forum.com/t/should-finds-be-in-the-controller-or-the-model-slimming-fat-controllers/118104/6 "2007-10-24T15:37:42Z")

</div>

On 10/24/07, Bob C. [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid) wrote:

> In Rails is it a common best practice to keep the find in the model even if  
> it is adding some complexity in certain cases, or should I just use whatever  
> makes sense for the situation?

The latter. There’s no general rule that says “find belongs in the  
model”; That’s cargo-culting. There’s nothing wrong with this  
controller method:

def list  
@widgets = Widget.find :all, :order =\> ‘created\_at’  
end

It would be silly to extract a model method for that.

For a real-world example of what to avoid, see my little article at  
[http://www.robertshowalter.com/articles/2007/06/21/controller-code-smell-in-the-wild](http://www.robertshowalter.com/articles/2007/06/21/controller-code-smell-in-the-wild)