Daily Refactor #4: Extract Method in the BulkTimeEntriesHelper – options generator

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

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

# 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 = ''
    unless open_issues.empty?
      html << ""
      html << options_from_collection_for_select(open_issues, :id, :to_s)
      html << ""
    end
 
    unless closed_issues.empty?
      html << ""
      html << options_from_collection_for_select(closed_issues, :id, :to_s)
      html << ""
    end
    html
  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

# 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 = ''
    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 = ""
    html << options_from_collection_for_select(collection, :id, :to_s)
    html << ""
    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.

Reference commit.