Refactoring on Rails: Multiple Scopes in Controller

I think I'd expect the natural progression of adding common code to ApplicationController to lead to two further refactoring, but neither is Rail-centric, and I've seen minor instances of both in my code so far.  I'm less sure about ApplicationHelper as I haven't had to add to much there yet.

Method one for dealing with an ever growing ApplicationController -- intermediary sub-classed controllers.  If you have code shared by a few, but not all controllers, and the controllers that share code segment themelves into related areans, this works.  Ie If controller A and B share code, as do C and D, but there are no "cross terms" -- no A and C/D or B and C/D.  This however is just a normal refactoring out of Fowler's book -- pushing shared code to the proper level of an inheritence hierarchy and/or introducing a new level.

Method two would be more Ruby-ish and would invovled placing shared code into a module that is then included into the appropriate place in the Controller hierarchy.  I think long term this is the more 'correct' refactoring, but its not quite as simple and I haven't really examined how hard/easy it is to test the resulting module.  "Repeating" the "include foo" in multiple controllers wouldn't appear non-DRY to me, rather than putting "include foo" into the ApplicationController, when other controllers might not need it (the Refused Bequest code smell, to me, is worse than this instance of Duplicated Code).

From looking at the Rails source it obvisions that they use Method two a lot....

I tend to be very anti-"Refused Bequest"...  If I had 10 controllers and 9 needed some common code, I'd stick it into an intermediate subclass.  (I commonly end up with an "AuthenticatedController" that enforces all the user-must-be-logged-in checks for the underlying controllers, with only 1-2 controllers descending directly from ApplicationController).  I haven't ended up with too much other shared code in controllers, as most of it ends up in the models....  I haven't had a lot of "Aspect"-esque things to scatter-shot around the controllers which I think is where the inheritence breaks down and the module approach wins...

I think a slightly more complicated, but perhaps cleaner approach for very large projects would be to stick a lot of the code into a module, that's included by ApplicationController, but enabled/configured by the subclasses.  Similar to the way ActiveRecord works -- all classes have the methods defined for "has_many", or "validates_x" but you have to "enable" it in the specific subclass, so I'd take a similar appraoch and create the libraries of shared, generic controller code that all controllers have the ability to activate.

I'm not sure I'm understanding you correctly. From what I can tell you have specific controller code that goes with a partial, and you would like to render this partial in different controllers/actions. This leads to duplication because the partial requires some code in the controller.

There are many ways to solve this problem, and it largely depends upon your specific situation for the best solution.

1. Sometimes you can move the controller's code into the model. For example, if you are doing a complex find, create a custom find method in your model and just call this directly in the view.

2. Move the controller code into a helper method and have that helper method render the partial at the end. You can then just call this helper method whenever you want to render the partial.

3. Move the common code into a before filter.

4. Use components. These are "mini" controllers that go along with partials. There are mixed opinions about them, but if none of the above options are satisfactory, it may be the best solution. Use it if it works for you.

It is difficult to give a recommendation because there are so many variables. A lot of it depends upon how many widgets there will be, how you plan to add/maintain widgets, and if you will have  controllers which these widgets will be mimicking. For example, if you have other developers adding their own widgets, the widgets would probably need to stand up on their own without relying on something already implemented within the application. On the other hand, if the widgets are just offering a "miniature" version of an existing full page controller, then you would want to integrate them more with the rest of the application.

Regarding user input, you can easily communicate to any other controller through AJAX. For example, let's say you have a "subscription" widget which accepts an email address. You also have a SubscriptionsController which manages these subscriptions. You can place the widget in a partial:

# in views/widgets/_subscription.rhtml
<div id="subscription" class="widget">
<%= error_messsages_for :subscription %>
<%= form_remote_tag :controller => 'subscriptions', :action => 'create' %>
  <p>Email Address: <%= text_field :subscription, :email %></p>
  <%= submit_tag 'Subscribe' %>
<%= end_form_tag %>
</div>

This partial can be rendered anywhere. You can then respond appropriately through RJS in the controller:

# in subscriptions/create.rjs
if @subscription.errors.empty?
  page[:subscription].replace_html = "Successfully subscribed."
else
  page[:subscription].replace :partial => 'widgets/subscription'
end

The reason I recommend using the primary controller for accepting widget input is in case you want to add subscriptions through some other method, say a traditional subscription page. If you had moved the subscription creation logic into its own "widget" controller/component, you may need to duplicate the logic between the primary controller and the component. If you won't need that functionality, then there are probably better solutions.

As for the widgets which require logic to display them, components seem like a reasonable solution. If you are looking for an alternative you may want to go with the second option I gave (helpers). I would probably do something like this:

module WidgetsHelper
  def recent_topics_widget(limit = 5)
    topics = Topic.find(:all, :order => 'created_at DESC', :limit => limit)
    render :partial => 'widgets/recent_topics', :locals => { :topics => topics }
  end
  
  # all other widgets go here
end
# in application.rb
helper :widgets # loads the WidgetsHelper for all views
# in views/widgets/_recent_topics.rhtml
<div id="recent_topics" class="widget">
<% for topic in topics %>
  <div class="topic"><%= topic.name %></div>
<% end %>
</div>

You can then call this helper method in any view to render the widget:

<%= recent_topics_widget(10) %>

As I said above, it all depends upon the details of your application. You may need widgets to be fully self-contained, in that case components are probably the best solution.

Edit: I should mention, components aren't directly accessible through the URL, so there's really no way they can respond directly to user input without going through a controller. If you don't want to give each widget which accepts user input its own controller, then you may want to build your own abstract widget management system. Each widget could have its own class/module which contains all of the appropriate logic so it is completely self contained. You can create a generic "widgets" controller which handles all user input and forwards it to the appropriate widget.

This is going beyond my experience as I've never had to do anything like it, so take it with a grain of salt.

Last edited by ryanb (2006-11-22 17:41:00)