CSC/ECE 517 Fall 2023 - E2375. Refactor classes relating to signing up for topics
Introduction
When managing sign-ups within expertiza, the evolution of functionality often leads to a need for refactoring existing code. In this case, several classes—sign_up_sheet.rb, sign_up_topic.rb, and signed_up_team.rb—require restructuring to streamline their functionalities and ensure they align with the current project landscape. This refactoring endeavor aims to declutter, reorganize, and enhance code readability and maintainability by removing redundant functionalities and repositioning methods in classes where they serve their intended purposes more explicitly. The end goal is a more coherent and efficient codebase that reflects the current needs of the project while laying the groundwork for future enhancements.
Methods to be refactored
Methods in sign_up_sheet.rb file
(1) self.signup_team
Code:
def self.signup_team(assignment_id, user_id, topic_id = nil) users_team = SignedUpTeam.find_team_users(assignment_id, user_id) if users_team.empty? # if team is not yet created, create new team. # create Team and TeamNode team = AssignmentTeam.create_team_with_users(assignment_id, [user_id]) # create SignedUpTeam confirmationStatus = SignUpSheet.confirmTopic(user_id, team.id, topic_id, assignment_id) if topic_id else confirmationStatus = SignUpSheet.confirmTopic(user_id, users_team[0].t_id, topic_id, assignment_id) if topic_id end ExpertizaLogger.info "The signup topic save status:#{confirmationStatus} for assignment #{assignment_id} by #{user_id}" confirmationStatus end
Code smells:
- Conditional Complexity: There's conditional complexity within the method based on whether users_team is empty or not, leading to slightly repetitive code.
- Lack of Clear Separation of Concerns: The method handles team creation, sign-up confirmation, and logging within the same block, which might violate the single responsibility principle.
- Magic Numbers/Variables: users_team[0].t_id uses index 0 directly, assuming there's always at least one entry in users_team.
- Change method name to Signup_sheet.
What to improve
- Error Handling: Add error handling for potential failures during team creation or sign-up processes.
(2) self.confirmTopic
Code:
def self.confirmTopic(user_id, team_id, topic_id, assignment_id) # check whether user has signed up already user_signup = SignUpSheet.otherConfirmedTopicforUser(assignment_id, team_id) users_team = SignedUpTeam.find_team_users(assignment_id, user_id) team = Team.find(users_team.first.t_id) if SignedUpTeam.where(team_id: team.id, topic_id: topic_id).any? return false end sign_up = SignedUpTeam.new sign_up.topic_id = topic_id sign_up.team_id = team_id result = false if user_signup.empty? # Using a DB transaction to ensure atomic inserts ApplicationRecord.transaction do # check whether slots exist (params[:id] = topic_id) or has the user selected another topic team_id, topic_id = create_SignUpTeam(assignment_id, sign_up, topic_id, user_id) result = true if sign_up.save end else # This line of code checks if the "user_signup_topic" is on the waitlist. If it is not on the waitlist, then the code returns # false. If it is on the waitlist, the code continues to execute. user_signup.each do |user_signup_topic| return false unless user_signup_topic.is_waitlisted end # Using a DB transaction to ensure atomic inserts ApplicationRecord.transaction do # check whether user is clicking on a topic which is not going to place him in the waitlist result = sign_up_wailisted(assignment_id, sign_up, team_id, topic_id) end end result end
Code smells:
- Method Length: The method is quite lengthy and performs multiple actions. It might benefit from breaking down into smaller, more focused methods.
- Complex Conditional Flow: The conditional flow involving transactions and multiple conditions could be challenging to follow and understand at first glance.
Design Considerations:
- Separation of Concerns: Ensure clear separation of concerns within the method. Each section of code should handle a specific aspect of the logic.
Refactoring and Code Enhancements:
- Extract Conditional Blocks into Methods: Break down conditional blocks into smaller methods to improve readability and maintainability. For instance, separate logic for handling cases where the user has already signed up or is on the waitlist.
- Improve Variable Naming: Use more descriptive variable names for better readability and understanding of the code. For example, renaming result, sign_up, user_signup, and user_signup_topic to more descriptive names would enhance clarity.
Team
Mentor
- Ed Gehringer
Members
- Dinesh Pasupuleti <dpasupu@ncsu.edu>
- Prateek Singamsetty <pksingam@ncsu.edu>
- Rushabh Shah <rshah32@ncsu.edu>