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.