Daily Refactor #51: Lazy Load Issues in Kanban

This change isn’t a strict refactoring since it’s adding a new feature, but since it was done with the intent of cleaning up the code I’m going to let it pass…. this time.

The Refactoring

The big problem with Kanban is that all callers have to use Kanban#find in order to setup the data. This meant:

  1. a simple Kanban.new call wouldn’t have any data, and
  2. there was no way to load just one or two panes of data (it’s all or nothing)

This coupled all callers to Kanban#find and the performance of that method.

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/kanban.rb
class Kanban
  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
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
40
41
42
43
44
45
46
47
48
49
50
51
52
53

# app/models/kanban.rb
class Kanban
  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
 
    @settings = Setting.plugin_redmine_kanban
    @users = get_users
  end
 
  def self.find
    Kanban.new
  end
 
  def incoming_issues
    @incoming_issues ||= incoming_pane.get_issues
  end
 
  def quick_issues
    @quick_issues ||= quick_pane.get_issues
  end
 
  def backlog_issues
    quick_issues # Needs to load quick_issues
    @backlog_issues ||= backlog_pane.get_issues(:exclude_ids => quick_issue_ids)
  end
 
  def selected_issues
    @selected_issues ||= KanbanIssue.find_selected
  end
 
  def active_issues
    @active_issues ||= active_pane.get_issues(:users => get_users)
  end
 
  def testing_issues
    @testing_issues ||= testing_pane.get_issues(:users => get_users)
  end
 
  def finished_issues
    @finished_issues ||= finished_pane.get_issues
  end
 
  def canceled_issues
    @canceled_issues ||= canceled_pane.get_issues
  end
 
end

Review

Basically what I did was to move all of the data access to the accessors, so no data would be loaded until it was tried to be accessed (called Lazy Load). This separated the Kanban initialization from the data loading and makes it’s easier to only load up one or two panes of data.

I thought this would be a huge performance upgrade since the Kanban board only updates one or two panes of data at a time. Sadly, it looks like there are other performance issues so I saw less than a 1% performance improvement. But I decided to keep this change since it makes the code easier to read. Performance improvements can be done later, once the code is well factored.

Reference commit