Daily Refactor #41: Consolidate Duplicate Conditional Fragments in StuffToDo

The Refactoring

Today I refactored the conditional of StuffToDo#conditions_for_available using a little known library in Redmine called ARCondition.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13

# app/models/stuff_to_do.rb
class StuffToDo < ActiveRecord::Base
  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

After

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

class StuffToDo < ActiveRecord::Base
  def self.conditions_for_available(filter_by, record_id)
    conditions_builder = ARCondition.new
    conditions_builder.add(["#{IssueStatus.table_name}.is_closed = ?", false ])
 
    case filter_by
    when :user
      conditions_builder.add(["assigned_to_id = ?", record_id])
    when :status
      conditions_builder.add(["#{IssueStatus.table_name}.id = (?)", record_id])
    when :priority
      conditions_builder.add(["#{Enumeration.table_name}.id = (?)", record_id])
    end
 
    conditions_builder.conditions
  end
end

Review

Using ARCondition, I was able split up how the conditions where being generated. So instead of creating three similar Arrays, I can create separate Arrays for each part of the conditions. Redmine uses this pattern in several places where the conditions need to be generated dynamically.

There is one more refactoring I want to try from the flay report, by combining the Redmine core patches to Issue and Project.

Reference commit