Daily Refactor #40: Extract Method in StuffToDo#available

The Refactoring

Working on the flay report for StuffToDo I tackled the #available method this morning.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27

# app/models/stuff_to_do.rb
class StuffToDo  :status,
                          :conditions => ["assigned_to_id = ? AND #{IssueStatus.table_name}.is_closed = ?",user.id, false ],
                          :order => 'created_on DESC')
    elsif filter[:status]
      status = filter[:status]
      issues = Issue.find(:all,
                          :include => :status,
                          :conditions => ["#{IssueStatus.table_name}.id = (?) AND #{IssueStatus.table_name}.is_closed = ?", status.id, false ],
                          :order => 'created_on DESC')
    elsif filter[:priority]
      priority = filter[:priority]
      issues = Issue.find(:all,
                          :include => [:status, :priority],
                          :conditions => ["#{Enumeration.table_name}.id = (?) AND #{IssueStatus.table_name}.is_closed = ?", priority.id, false ],
                          :order => 'created_on DESC')
    elsif filter[:projects]
      # TODO: remove 'issues' naming
      issues = active_and_visible_projects.sort
    end
    next_issues = StuffToDo.find(:all, :conditions => { :user_id => user.id }).collect(&:stuff)
 
 
    return issues - next_issues
  end
 
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39

class StuffToDo  :status,
                          :conditions => conditions_for_available(:user, user.id),
                          :order => 'created_on DESC')
    elsif filter[:status]
      status = filter[:status]
      issues = Issue.find(:all,
                          :include => :status,
                          :conditions => conditions_for_available(:status, status.id),
                          :order => 'created_on DESC')
    elsif filter[:priority]
      priority = filter[:priority]
      issues = Issue.find(:all,
                          :include => [:status, :priority],
                          :conditions => conditions_for_available(:priority, priority.id),
                          :order => 'created_on DESC')
    elsif filter[:projects]
      # TODO: remove 'issues' naming
      issues = active_and_visible_projects.sort
    end
    next_issues = StuffToDo.find(:all, :conditions => { :user_id => user.id }).collect(&:stuff)
 
 
    return issues - next_issues
  end
 
  private
 
  def self.conditions_for_available(filter_by, record_id)
    case filter_by
    when :user
      ["assigned_to_id = ? AND #{IssueStatus.table_name}.is_closed = ?", record_id, false ]
    when :status
      ["#{IssueStatus.table_name}.id = (?) AND #{IssueStatus.table_name}.is_closed = ?", record_id, false ]
    when :priority
      ["#{Enumeration.table_name}.id = (?) AND #{IssueStatus.table_name}.is_closed = ?", record_id, false ]
    end
  end
 
end

Review

I really like these kind of refactorings. They make it easy to see the similar areas of code and make the next refactorings easy. Next I could refactor the entire finder in #available or this new #conditions_for_available.

Reference commit