Rails - Oct 2007 - Should Finds be in the controller or the model? (slimming fat controllers)

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.


--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

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

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

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


On Oct 23, 5:04 pm, "Michael Graff"
<skan.gryp...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
> 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

--~--~---------~--~----~------------~-------~--~----~
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''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?




On 10/24/07, randomutterings...
<randomutterings-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
>
>
> 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
>
>
> On Oct 23, 5:04 pm, "Michael Graff"
<skan.gryp...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 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
>
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
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 10/24/07, Bob Clewell
<bobclewell-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
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

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

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.

On Oct 24, 9:36 am, "Bob Showalter"
<showa...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
> On 10/24/07, Bob Clewell
<bobclew...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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
athttp://www.robertshowalter.com/articles/2007/06/21/controller-code-sm...

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---