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