Today Devver team showed me a new feature they added to their Caliper project, the flay code view. I liked it so much I used it on today's BulkTimeEntry plugin refactoring.
The Refactoring
Flay is a tool that checks for duplicated Ruby code. The BulkTimeEntry plugin only showed one piece of code that was duplicated but it's perfect for a simple extract method.
Before
# bulk_time_entries_helper.rb
def grouped_options_for_issues(issues)
open_issues = []
closed_issues = []
issues.each do |issue|
if issue.closed?
closed_issues << issue
else
open_issues << issue
end
end
html = '<option></option>'
unless open_issues.empty?
html << "<optgroup label='#{l(:label_open_issues)}'>"
html << options_from_collection_for_select(open_issues, :id, :to_s)
html << "</optgroup>"
end
unless closed_issues.empty?
html << "<optgroup label='#{l(:label_closed_issues)}'>"
html << options_from_collection_for_select(closed_issues, :id, :to_s)
html << "</optgroup>"
end
html
end
After
# bulk_time_entries_helper.rb
def grouped_options_for_issues(issues)
open_issues = []
closed_issues = []
issues.each do |issue|
if issue.closed?
closed_issues << issue
else
open_issues << issue
end
end
html = '<option></option>'
unless open_issues.empty?
html << labeled_option_group_from_collection_for_select(:label_open_issues, open_issues)
end
unless closed_issues.empty?
html << labeled_option_group_from_collection_for_select(:label_closed_issues, closed_issues)
end
html
end
def labeled_option_group_from_collection_for_select(label, collection)
html = "<optgroup label='#{l(label)}'>"
html << options_from_collection_for_select(collection, :id, :to_s)
html << "</optgroup>"
html
end
Review
This was a really easy refactor since the method generated the same HTML string but using a different label and collection. There are still a few minor style issues that I'll tackle next time.
If you haven't used Caliper yet, I'd recommend you give it a try. I have Redmine setup with a post commit hook so it's metrics are generated every time we commit. Having an extra hand watching code quality is always helpful.
