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 |
# 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 |
# 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 |
# 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 |
# 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 |
# 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 |
# 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