The Refactoring
Since creating the new KanbanPane class, there has been some wrapper methods to access Kanban’s private utility methods. This refactoring uses move method to move that utility method from Kanban to KanbanPane.
Before
1
2
3
4
5
6
7
8
9
10
11
12
|
# app/models/kanban.rb
class Kanban
def missing_settings(pane, options={})
skip_status = options.delete(:skip_status)
@settings.blank? ||
@settings['panes'].blank? ||
@settings['panes'][pane].blank? ||
@settings['panes'][pane]['limit'].blank? ||
(@settings['panes'][pane]['status'].blank? && !skip_status)
end
end |
# app/models/kanban.rb
class Kanban
def missing_settings(pane, options={})
skip_status = options.delete(:skip_status)
@settings.blank? ||
@settings['panes'].blank? ||
@settings['panes'][pane].blank? ||
@settings['panes'][pane]['limit'].blank? ||
(@settings['panes'][pane]['status'].blank? && !skip_status)
end
end
1
2
3
4
5
6
7
8
9
|
# app/models/kanban_pane.rb
class KanbanPane
# 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 |
# app/models/kanban_pane.rb
class KanbanPane
# 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
After
1
2
3
4
|
# app/models/kanban.rb
class Kanban
# ...
end |
# app/models/kanban.rb
class Kanban
# ...
end
1
2
3
4
5
6
7
8
9
10
11
12
|
# app/models/kanban_pane.rb
class KanbanPane
def missing_settings(pane, options={})
skip_status = options.delete(:skip_status)
settings.blank? ||
settings['panes'].blank? ||
settings['panes'][pane].blank? ||
settings['panes'][pane]['limit'].blank? ||
(settings['panes'][pane]['status'].blank? && !skip_status)
end
end |
# app/models/kanban_pane.rb
class KanbanPane
def missing_settings(pane, options={})
skip_status = options.delete(:skip_status)
settings.blank? ||
settings['panes'].blank? ||
settings['panes'][pane].blank? ||
settings['panes'][pane]['limit'].blank? ||
(settings['panes'][pane]['status'].blank? && !skip_status)
end
end
Review
Move method turned out to get a good refactoring for this code. Not only did it put #missing_settings into the class where it belongs, but it also helped to get rid of the ugly #send hack that KanbanPane was using. There is another similar method I need to take care of tomorrow.
Reference commit