Feature churn can really get bogged down in older Rails projects. One of the reasons is action packed controllers: the art of stacking actions on a controller that should not have those actions. Let’s look at a few ways to identify these and get rid of them.
Sticking to guidelines
People have adopted a number of simple guidelines to keep the scope of controllers small. One of these is only using index, new, create, show, edit, update, destroy actions on the controller. When you need any actions other than these you’re probably packing on too much functionality.
resources :books do member do delete :delete_cover end end class BooksController < ApplicationController def new @book = Book.new end def delete_cover @book = Book.find(params[:id]) if @book.cover.destroy redirect_to @book end end end
In the example above we’re packing functionality related to covers on top of the books controller.
Fixing the problem
Create a new controller specifically for the model you were operating on. Notice how this also cleans up the routes.
resources :books do resource :cover end class CoversController < ApplicationController def destroy @book = Book.find(params[:book_id]) if @book.cover.destroy redirect_to @book end end end
You might think: awesome, anyone can refactor a controller with an obvious solution but my controller is a cesspit worse than the atmosphere of Jupiter.
class BooksController < ApplicationController before_action :find_book_for_author, except: :edit_book before_action :disallow_offenders before_action :set_ccr_rules_header, only: :new layout :set_layout_for_author def delete_cover if @book.cover.destroy redirect_to @book end end private def find_book_for_author @author = Author.find(session[:current_author_id]) @book = @author.books.find(param[:id]) end # […] etc etc end
Refactoring always happens with baby steps. The solution here is to embrace the cesspit and think about fixing it later.
# Contains all methods shared by the books # and covers controller. module Concerns::BooksAndCoversControllerMethods protected def find_book_for_author @author = Author.find(session[:current_author_id]) @book = @author.books.find(book_id_param) end # […] etc etc end class CoversController < ApplicationController include Concerns::BooksAndCoversControllerMethods before_action :find_book_for_author before_action :disallow_offenders layout :set_layout_for_author def destroy if @book.cover.destroy redirect_to @book end end private def book_id_param params[:book_id] end end
Even though all the ugly before actions are still there we’ve constrained them to one source file. Our original goal of simplifying the books controller and the routes has been achieved.
Usually when you split out code like this you will get some ideas on how to refactor it further. Don’t worry if you don’t have an epiphany immediately. You can leave the code and think about a great solution in the shower.