Daily Refactor #68: Extract Class to New Controller

The Refactoring

Today I used extract class to move the #calendar method from IssuesController to it’s own controller. This is part of a large refactoring plan I have to get IssuesController slimmed down to a more manageable size.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

# app/controllers/issues_controller.rb
class IssuesController  [:index, :changes, :calendar, :preview, :context_menu]
  before_filter :find_optional_project, :only => [:index, :changes, :calendar]
 
  def calendar
    if params[:year] and params[:year].to_i > 1900
      @year = params[:year].to_i
      if params[:month] and params[:month].to_i > 0 and params[:month].to_i  [:tracker, :assigned_to, :priority],
                              :conditions => ["((start_date BETWEEN ? AND ?) OR (due_date BETWEEN ? AND ?))", @calendar.startdt, @calendar.enddt, @calendar.startdt, @calendar.enddt]
                              )
      events += @query.versions(:conditions => ["effective_date BETWEEN ? AND ?", @calendar.startdt, @calendar.enddt])
 
      @calendar.events = events
    end
 
    render :layout => false if request.xhr?
  end
end

After

1
2
3
4
5
6

# app/controllers/issues_controller.rb
class IssuesController  [:index, :changes, :preview, :context_menu]
  before_filter :find_optional_project, :only => [:index, :changes]
 
  # No more calendar action
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

# app/controllers/calendars_controller.rb
class CalendarsController  :query_statement_invalid
 
  helper :issues
  helper :projects
  helper :queries
  include QueriesHelper
 
  def show
    if params[:year] and params[:year].to_i > 1900
      @year = params[:year].to_i
      if params[:month] and params[:month].to_i > 0 and params[:month].to_i  [:tracker, :assigned_to, :priority],
                              :conditions => ["((start_date BETWEEN ? AND ?) OR (due_date BETWEEN ? AND ?))", @calendar.startdt, @calendar.enddt, @calendar.startdt, @calendar.enddt]
                              )
      events += @query.versions(:conditions => ["effective_date BETWEEN ? AND ?", @calendar.startdt, @calendar.enddt])
 
      @calendar.events = events
    end
 
    render :layout => false if request.xhr?
  end
 
 
end

Review

Since I just finished up refactoring the GanttsController this refactoring was pretty easy. IssuesController is a still a large controller (482 lines) but it’s getting better each week. It looks like all of the remaining actions in IssuesController belong there so I’m going to start to tackle some duplication and code smells inside each method.

Reference commit