CSC/ECE 517 Fall 2019 - Project E1943. Refactor sign up sheet controller.rb
Refactor sign up sheet controller.rb
This page gives a description of the refactoring done in the sign_up_sheet_controller.rb as part of the OSS project.
(NOTE: This wiki is not final and is still a work in progress.)
About Expertiza
Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
Description of the project
The following is an Expertiza based OSS project which deals with the signup_sheet_controller.rb. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. The goal of this project is to attempt to make this part of the application easier to read and maintain.
About Signup Sheet Controller
This controller is used while signing up for a topic from the signup sheet. This controller allows users to signup as an instructor as well as a student. An instructor has the authority to do CRUD operations on topics and enrolled/waitlisted members of each topic. The instructor can also add or update deadlines for the assignment.
Issues and Solutions
- Issue 1 - Inconsistent naming. When used as a verb, “sign up” is two words. When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.” Cf.“sign_up_as_instructor”.
- Solution- Changed the name of the controller from sign_up_sheet_controller.rb --> signup_sheet_controller.rb. Also changed several method names that had 'sign_up' being used as an adjective or a noun to 'signup'.
- Issue 2 - Create method has and if-else condition determining if create or update should be called. Create method should not be responsible for calling update.
- Solution-Created another method 'call_create_or_update' which has the if-else condition that determines if create or update is called.
- Issue 3 - Update method has a plethora of instance variables defined before updating. These might not be necessary. Decide whether so many instance variables are really needed.
- Solution-Used a single update_attributes method in place of the many instance variables.
- Issue 4 - Destroy has a misleading else flash message.
- Solution-Changed the misleading flash message in the else condition to be more straightforward.
- Issue 5 - Add_signup_topics_staggered does not do anything. Find out why add_signup_topics_staggered simply calls add_signup_topics, and why separate functions are needed.
- Solution-It was found that the method add_signup_topics_staggered was unnecessary and was removed.
- Issue 6 - Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)
- Solution-Several method names were appropriately improved and renamed.
- Issue 7 - What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed.
- Solution-It was found that both the signup_as_instructor and signup_as_instructor_action methods are needed and their use was explained with proper comments.
- Issue 8 - The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.
- Solution-Provided comments for blocks of code and created helper methods.
- Issue 9 - Refactor participants variable in load_add_signup_topics
- Solution-Refactored the participants variable and improved readability.
- Issue 10 - Signup_as_instructor_action has a complicated if-else ladder.
- Solution-The if-else ladder was made more elegant and readable.
- Issue 11 - Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle.
- Solution-The excess code in both the delete_signup and delete_signup_as_instructor methods was removed and replaced by just one 'get_status' method call.
Code Improvements
References
- Rspec Documentation
- Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin