The Refactoring
Going back to the flay report, I used a combination of extract method and move method to move some duplication from a Controller to a shared Model method.
Before
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
| # app/controller/users_controller.rb
class UsersController @user)
@membership.attributes = params[:membership]
@membership.save if request.post?
respond_to do |format|
format.html { redirect_to :controller => 'users', :action => 'edit', :id => @user, :tab => 'memberships' }
format.js {
render(:update) {|page|
page.replace_html "tab-content-memberships", :partial => 'users/memberships'
page.visual_effect(:highlight, "member-#{@membership.id}")
}
}
end
end
end |
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
| # app/controller/groups_controller.rb
class GroupsController @group)
@membership.attributes = params[:membership]
@membership.save if request.post?
respond_to do |format|
format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'memberships' }
format.js {
render(:update) {|page|
page.replace_html "tab-content-memberships", :partial => 'groups/memberships'
page.visual_effect(:highlight, "member-#{@membership.id}")
}
}
end
end
end |
1
2
3
4
| # app/models/member.rb
class Member < ActiveRecord::Base
# ..
end |
After
1
2
3
4
5
6
7
8
9
10
11
| # app/controller/users_controller.rb
class UsersController 'users', :action => 'edit', :id => @user, :tab => 'memberships' }
format.js {
render(:update) {|page|
page.replace_html "tab-content-memberships", :partial => 'users/memberships'
page.visual_effect(:highlight, "member-#{@membership.id}")
}
}
end
end
end |
1
2
3
4
5
6
7
8
9
10
11
| # app/controller/groups_controller.rb
class GroupsController 'groups', :action => 'edit', :id => @group, :tab => 'memberships' }
format.js {
render(:update) {|page|
page.replace_html "tab-content-memberships", :partial => 'groups/memberships'
page.visual_effect(:highlight, "member-#{@membership.id}")
}
}
end
end
end |
1
2
3
4
5
6
| # app/models/member.rb
class Member principal)
@membership.attributes = new_attributes
@membership
end
end |
Review
This was a simple refactoring that shows how you can combine extract method and move method in Rails to move duplicated code out of the controllers and into the models.
Reference commit
Share
Related posts:
- Redmine Refactor #91: Pull Up set_flash_from_bulk_issue_save to ApplicationController
- Redmine Refactor #90: Pull Up Method to ApplicationController
- Daily Refactor #67: Pull Up Method to ApplicationController
- Daily Refactor #66: Pull Up Method to ApplicationController
- Daily Refactor #35: Pull up method for finding a controller’s model object
About Eric Davis
I founded
Little Stream Software where I provide Redmine and ChiliProject services to help projects teams. I also created an ebook,
Redmine Tips, were I show you how to become more productive using Redmine.
I am also the author of Refactoring Redmine, where I go about
refactoring Rails using Redmine as an example.
