Daily Refactor #50: Remove Middle Man methods in Kanban

In yesterday’s post I used Replace Data Value with Object to start converting much of the Kanban plugin’s code to be more object oriented. I finished up that refactoring in some other commits if you’re interested in how it went.

The Refactoring

As a result of this refactoring, I was left with a bunch of wrapper methods that would just delgate method calls over to the new KanbanPane classes. This made sense when the main Kanban class was managing the data but now it’s just overhead. Using Remove Middle Man, I threw away a lot of those wrapper methods and accessed the KanbanPane directly.

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
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65

# app/models/kanban.rb
class Kanban
  def self.find
    kanban = Kanban.new
    kanban.settings = Setting.plugin_redmine_kanban
    kanban.users = kanban.get_users
    kanban.incoming_issues = kanban.get_incoming_issues
    kanban.quick_issues = kanban.get_quick_issues
    kanban.backlog_issues = kanban.get_backlog_issues(kanban.quick_issues.to_a.flatten.collect(&:id))
    kanban.selected_issues = KanbanIssue.find_selected
    kanban.active_issues = kanban.get_active
    kanban.testing_issues = kanban.get_testing
    kanban.finished_issues = kanban.get_finished_issues
    kanban.canceled_issues = kanban.get_canceled_issues
    kanban
  end
 
  def get_incoming_issues
    get_issues_for_pane(:incoming)
  end
 
  def get_backlog_issues(exclude_ids=[])
    get_issues_for_pane(:backlog, :exclude_ids => exclude_ids)
  end
 
  def get_quick_issues
    get_issues_for_pane(:quick)
  end
 
  def get_finished_issues
    get_issues_for_pane(:finished)
  end
 
  def get_canceled_issues
    get_issues_for_pane(:canceled)
  end
 
  def get_active
    KanbanPane::ActivePane.new.get_issues(:users => @users)
  end
 
  def get_testing
    KanbanPane::TestingPane.new.get_issues(:users => @users)
  end
 
  private
 
  def get_issues_for_pane(pane, options = {})
    case pane
    when :finished
      KanbanPane::FinishedPane.new.get_issues(options)
    when :canceled
      KanbanPane::CanceledPane.new.get_issues(options)
    when :quick
      KanbanPane::QuickPane.new.get_issues(options)
    when :backlog
      KanbanPane::BacklogPane.new.get_issues(options)
    when :incoming
      KanbanPane::IncomingPane.new.get_issues(options)
    else
      return [[]]
    end
 
  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

# app/models/kanban.rb
class Kanban
  attr_reader :incoming_pane, :backlog_pane, :quick_pane, :canceled_pane, :finished_pane, :active_pane, :testing_pane
 
  def initialize
    @incoming_pane = KanbanPane::IncomingPane.new
    @backlog_pane = KanbanPane::BacklogPane.new
    @quick_pane = KanbanPane::QuickPane.new
    @canceled_pane = KanbanPane::CanceledPane.new
    @finished_pane = KanbanPane::FinishedPane.new
    @active_pane = KanbanPane::ActivePane.new
    @testing_pane = KanbanPane::TestingPane.new
  end
 
  def self.find
    kanban = Kanban.new
    kanban.settings = Setting.plugin_redmine_kanban
    kanban.users = kanban.get_users
    kanban.incoming_issues = kanban.incoming_pane.get_issues
    kanban.quick_issues = kanban.quick_pane.get_issues
    kanban.backlog_issues = kanban.backlog_pane.get_issues(:exclude_ids => kanban.quick_issue_ids)
    kanban.selected_issues = KanbanIssue.find_selected
    kanban.active_issues = kanban.active_pane.get_issues(:users => kanban.users)
    kanban.testing_issues = kanban.testing_pane.get_issues(:users => kanban.users)
    kanban.finished_issues = kanban.finished_pane.get_issues
    kanban.canceled_issues = kanban.canceled_pane.get_issues
    kanban
  end
 
  # No more "get_incoming_issues", "get_backlog_issues", "get_quick_issues",
  # "get_finished_issues", "get_canceled_issues", "get_active", "get_testing",
  # or "get_issues_for_pane"
 
end

Review

By making Kanban manage it’s own KanbanPane, it becomes easy to change Kanban#find to use those panes directly to get the issues on them. This removes at least two levels of delegation and also makes the class more flexible.

Next I’d like to add a new feature I’ve been wanting, lazy loading the issues for each pane.

Reference commit