E1908 signupsheet

E1908. Refactoring the Sign-up sheet Controller

This page provides a description of the Expertiza based OSS project.

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.

Problem Statement

The following tasks were accomplished in this project:

• Improved the clarity of code by improving the variable and parameter names.
• Followed naming conventions throughout and renamed methods with inconsistent names including the calling methods.
• Rectified several unwanted if-else conditions in methods and optimized the code.
• Refactored all instance variables and removed unnecessarily defined variables.
• Removed certain unwanted flash messages that occur for some user actions.
• Included comments for functionalities throughout for better understanding.

About Sign-up sheet Controller

Sign-up sheet controller contains all functions related to management of the signup sheet for an assignment function to add new topics to an assignment, edit properties of a particular topic, delete a topic, etc are included here.

Drawbacks and Solutions

• Problem 1: Create method has an if-else condition determining if create or update should be called. Create method should not be responsible for calling update. Identify why the if-else condition exists. The if-else condition exists because the current implementation calls update if a signup sheet with the same name already exists.
• Solution: Rectified this method by removing the call to update and flashing an error instead.

• Problem 2: Update method has a plethora of instance variables defined before updating. These are not necessary (For e.g., look at update method of bookmarks_controller).
• Solution: Refactored the variables not needed out.

• Problem 4: The list method is too long and is sparsely commented.
• Solution: Added comments.

# function to list all topics and bids to a participant
  def list
    @participant = AssignmentParticipant.find(params[:id].to_i)
    @assignment = @participant.assignment
    @@ -155,6 +156,8 @@ def list
    @max_team_size = @assignment.max_team_size
    team_id = @participant.team.try(:id)

    # If the assignment supports bidding, add all the bids of an 
    # individual or team to the list of signed topics
    if @assignment.is_intelligent
      @bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
      signed_up_topics = []

• Problem 5: Add_signup_topics_staggered does not do anything.
• Solution: Renamed participants variable to 'teams'.

 #add_signup_topics_staggered calls add_signup_topics and does nothing else.
 def add_signup_topics_staggered
    add_signup_topics 
  end

• Problem 6: Several method names are renamed to be more intuitive.
• Solution: load_add_signup_topics is renamed to get_assignment_data and ad_info is renamed to get_ad.

  # Contains links that let an admin or Instructor edit, delete, view enrolled/waitlisted members for each topic
  # Also contains links to delete topics and modify the deadlines for individual topics. Staggered means that different topics can have different deadlines.
  def add_signup_topics
    load_add_signup_topics(params[:id])
    get_assignment_data(params[:id])
    SignUpSheet.add_signup_topic(params[:id])
  end

  @@ -109,7 +109,7 @@ def add_signup_topics_staggered
  end

  # retrieves all the data associated with the given assignment. Includes all topics,
  def load_add_signup_topics(assignment_id)
  def get_assignment_data(assignment_id)
    @id = assignment_id
    @sign_up_topics = SignUpTopic.where('assignment_id = ?', assignment_id)
    @slots_filled = SignUpTopic.find_slots_filled(assignment_id)
  @@ -374,7 +374,7 @@ def save_topic_deadlines
  # This method is called when a student click on the trumpet icon. So this is a bad method name. --Yang
  def show_team
    if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
      @results = ad_info(assignment.id, topic.id)
      @results = get_ad(assignment.id, topic.id)
      @results.each do |result|
        result.keys.each do |key|
          @current_team_name = result[key] if key.equal? :name

• Problem 7: Signup_as_instructor_action has if-else ladder.
• Solution: It has been made more elegant using a helper function.

• Problem 8: Delete_signup and delete_signup_as_instructor have much in common and violates the DRY principle.
• Solution: Refactored them by moving the duplicate code to a helper function.

• Problem 9: Participants variable in load_add_signup_topics actually means teams that signed up for a topic.
• Solution: Renamed participants variable to 'teams'.

    # to treat all assignments as team assignments
    # Though called participants, @participants are actually records in signed_up_teams table, which
    # is a mapping table between teams and topics (waitlisted recored are also counted)
    @participants = SignedUpTeam.find_team_participants(assignment_id, session[:ip])
    @teams = SignedUpTeam.find_team_participants(assignment_id, session[:ip])
  end

References

1. https://github.com/expertiza/expertiza
2. http://expertiza.ncsu.edu/ The live Expertiza website