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

Rails
1

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.

2

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

3

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

4

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?

5

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.

6

On 10/24/07, Bob C. [email protected] 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