Redmine Refactor #85: Extract Method in IssuesController#move

Whenever I work with legacy code I always turn to extract method and move method for most of my refactoring. Redmine’s IssuesController is no exception.

Using extract method on the #move action I was able to extract several lines of code deep in the method and also remove a temporary variable.

Before

1
2
3
4
5
6
7
8
9
10

class IssuesController  params, :issue => issue, :target_project => @target_project, :copy => !!@copy })
        if r = issue.move_to_project(@target_project, new_tracker, {:copy => @copy, :attributes => changed_attributes})
          moved_issues << r
        else
          unsaved_issue_ids << issue.id
        end
      end
      # ...
  end
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

class IssuesController  params, :issue => issue, :target_project => @target_project, :copy => !!@copy })
        if r = issue.move_to_project(@target_project, new_tracker, {:copy => @copy, :attributes => extract_changed_attributes_for_move(params)})
          moved_issues << r
        else
          unsaved_issue_ids << issue.id
        end
      end
      # ...
  end
 
  def extract_changed_attributes_for_move(params)
    changed_attributes = {}
    [:assigned_to_id, :status_id, :start_date, :due_date].each do |valid_attribute|
      unless params[valid_attribute].blank?
        changed_attributes[valid_attribute] = (params[valid_attribute] == 'none' ? nil : params[valid_attribute])
      end
    end
    changed_attributes
  end
end

This refactoring helps to remove some of the smells that metric_fu found with #move. It also let me remove the changed_attributes local variable, since it’s only used once outside of extract_changed_attributes_for_move.

Reference commit