Daily Refactor #49: Replace Incoming Pane’s Data Value with Object

I’ve made a mistake.

The last few refactorings I’ve been doing to the Kanban plugin have been useful but they haven’t really been improving the design of the plugin. I thought about it yesterday and discovered that I was just swapping out bits of procedural code for more procedural code. Once I saw this, I found a way to convert a lot of the procedural code into more object oriented code using Replace Data Value with Object.

The Refactoring

This refactoring creates a new class to store the behavior for one of the Kanban panes.

Before

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

# app/models/kanban.rb
class Kanban
  def get_issues_for_pane(pane, options = {})
    # ...
    when :incoming
      return [[]] if missing_settings('incoming')
 
      conditions.add ["status_id = ?", @settings['panes']['incoming']['status']]
 
      return Issue.visible.find(:all,
                                :limit => @settings['panes']['incoming']['limit'],
                                :order => "#{Issue.table_name}.created_on ASC",
                                :conditions => conditions.conditions)
    # ...
  end
end

After

1
2
3
4
5
6
7
8
9

# app/models/kanban.rb
class Kanban
  def get_issues_for_pane(pane, options = {})
    # ...
    when :incoming
      KanbanPane::IncomingPane.new.get_issues(options)
    # ...
  end
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

# app/models/kanban_pane.rb
class KanbanPane
  def settings
    Setting.plugin_redmine_kanban
  end
 
  def get_issues(options={})
    nil
  end
 
  private
 
  # TODO: Wrapper until moved from Kanban
  def missing_settings(pane, options={})
    kanban = Kanban.new
    kanban.settings = Setting.plugin_redmine_kanban
    kanban.send(:missing_settings, pane, options)
  end
end
 
1
2
3
4
5
6
7
8

# app/models/kanban_pane/incoming_pane.rb
class KanbanPane::IncomingPane  settings['panes']['incoming']['limit'],
                              :order => "#{Issue.table_name}.created_on ASC",
                              :conditions => conditions.conditions)
 
  end
 
end

Review

This is the kind of refactoring I’ve been looking for. By creating a new set of classes (KanbanPane, IncomingPane), I can start to advantage of Ruby’s object orientation. Once I finish extracting the other panes out from Kanban#get_issues_for_pane, I can start to move all of the other Pane behavior over to KanbanPane. This is going to help out in the long term, since I’m planning to add a few more Panes to Kanban in the future.

(I’m going to go ahead and do the other pane extractions today, so there will be some fresh code to look at tomorrow.)

Reference commit