I am working on application for tracking different issues. Each issue belongs to some category.
I want to display category list on each page in the footer.
Right now I am doing it like this:
class ApplicationController < ActionController::Base
@footer_categories = Category.all
And in footer section of application layout I have this code:
<%= render collection: @footer_categories, partial: 'footer_category' %>
But is this a correct way? Is it ok to initialize this instance variable for each controller action?
It doesn’t feel right to me, but I do not know better solution yet.
If you can provide me some best practice it would be great!
Many thanks in advance for your answers.
+100 to this question, it violates Sandi Metz’ rules, but other approaches seem bad as well.
I support the desire to create only a single instance variable in each controller. To that end, what’s the downside to ripping out all the controller code and changing the partial call to
<%= render collection: Category.all, partial: 'footer_category' %>
So it seems the problem there is that now your views and partials are handling the task of database calls. Granted, calling all records for a given table isn’t going to raise an exception, but what happens when the list of categories gets filtered down on a per user basis at some point in the future?
I’ve seen the Cells gem used for just this purpose: http://cells.rubyforge.org/. Cells allows you to have define a mini-controller that will be used to render just that part of the page.
I would be interested to see what the thoughtbot guys have to say about this.
One thing you could do, which is in the spirit of Sandi Metz’s rules, is to create a presenter. The idea is that the object you create in the controller represents all the variable state of the view. In this case, it would include the footer’s data and handle loading it from the database.
The other thing you can do, since the footer data isn’t really “controlled” by the controller it’s in, is have the view or a helper handle it. While the data is variable in that it might change over time, there is no logic surrounding it or the loading of it. If you find the idea of putting DB code in the view distasteful (and, trust me, I don’t blame you), then putting it into a helper might serve. If that doesn’t alleviate the smell for you, then a presentation object (as I mentioned above) would be the way to go.
Going further down the presenter route, an object solely to present the footer would make sense, and that object can then be composed by the presenter that handles the whole page. That way, you can encapsulate what little logic there is to loading the Categories and still have a presentable object to hand to the view.
The more I think of it, the more I like the idea of the composes presenter. Putting the logic into a helper is probably defensible, but I find myself already needing to rationalize even keeping it in this post, so maybe defending it to your colleagues would be more than you want to handle.
I think it’s handy to have one place that loads data need on every page. i’m not opposed to a before_filter, but i wouldn’t call it footer categories, i would do something more generic and load all common page data.
meanwhile, the important point to note is this is a perfect place for a cache! don’t hit the database each time, but rather cache this data and clear the cache as often as you think this list might get updated.