CSC/ECE 517 Spring 2015/oss E1509 lds
E1509. Refactoring SignUpController and SignUpSheetController
This page talks about an open source project based on Expertiza. As a part of contribution to Expertiza, this project aims to refactor SignUpController and SignUpSheetControllers.
Introduction to Expertiza
Expertiza is a web application developed by Ruby on Rails framework. It serves as a peer review system for professors and students at NC State and some other colleges and universities. Students can submit different assignments and peer-review reusable learning objects (articles, code, web sites, etc)<ref>https://github.com/expertiza/expertiza</ref>. The Expertiza project is supported by the National Science Foundation. And it is an open source application and the source code can be cloned on Github .
Problem Statement
Classes involved
* sign_up_sheet.rb * sign_up_topic.rb * assignments_controller.rb * response.rb * and possibly other model classes
What it does
Lists topics available for an assignment, checks whether a user is signed up for a topic, allows users to sign up for topics.
What's wrong with it
- These two controllers seem to do almost the same thing. They have many of the same methods. SignUpSheetController is much longer, and has many more recent mods, but some methods of SignUpController seem more sophisticated than SignUpSheetController. So, your first job is to figure out if both controllers are being used. If not, remove the unused controller. Or, move the functions to a single controller if that makes sense to do.
- Neither controller is at all RESTful; i.e.., its method names aren’t the standard names new, create, edit, delete, etc. Functionality is divided differently than in a standard controller.
a. def confirm_topic(creator_id, topic_id, assignment_id)
b. def delete_signup
c. def delete_signup_for_topic(assignment_id,topic_id)
d. def other_confirmed_topic_for_user(assignment_id, creator_id)
e. def signup
f. def slotAvailable?(topic_id)
- Functionality that should be in models is incorporated into the controller.
- Some methods are too long and appear to do more than one thing
- This class interfaces with assignments_controller so that a list of topics can be displayed when one is editing an assignment. Please be careful that your changes do not affect the functionality on the Topics tab of editing an assignment.
- Rename the controller(s) to SignupController and/or SignupSheetController. (“Sign up”, which gets written as SignUp in camel case, is a verb, whereas “Signup” is a noun.)
Modification We Made
Removing unused controller
RESTful style implementation
The purpose of the list method in the SignupSheet controller is to list all the topics in the specific assignments. According to RESTful rules, a method that returns a list of all available objects should be named as index. Therefore, we renamed the list method to the index method in the controller and in all the files that had references to the list method of the signup_sheet_controller.
Before Refactoring :
Method : list
def list \t@assignment_id = params[:id] @sign_up_topics = SignUpTopic.where( ['assignment_id = ?', params[:id]]).all @slots_filled = SignUpTopic.find_slots_filled(params[:id]) @slots_waitlisted = SignUpTopic.find_slots_waitlisted(params[:id]) @show_actions = true @priority = 0 assignment=Assignment.find(params[:id]) #end if assignment.due_dates.find_by_deadline_type_id(1)!= nil unless !(assignment.staggered_deadline? and assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now ) @show_actions = false end #Find whether the user has signed up for any topics; if so the user won't be able to #sign up again unless the former was a waitlisted topic #if team assignment, then team id needs to be passed as parameter else the user's id users_team = SignedUpUser.find_team_users(params[:id],(session[:user].id)) if users_team.size == 0 @selected_topics = nil else #TODO: fix this; cant use 0 @selected_topics = SignUpSheetController.other_confirmed_topic_for_user(params[:id], users_team[0].t_id) end SignUpTopic.remove_team(users_team, @assignment_id) end end
Moving inappropriate functionality from controller to model
Breaking down long methods
Deleting redundant button
Renaming controller
Following global rules
References
<references/>