CSC/ECE 517 Fall 2016/E1641. Refactor review mapping controller.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 20: Line 20:
# By searching the whole project and routes, we verify that methods delete_mappings in this controller are not used in anywhere.
# By searching the whole project and routes, we verify that methods delete_mappings in this controller are not used in anywhere.
# This function have many long lines. We already shorten each long line to multiple lines. And also, we split this long function into three different functions.
# This function have many long lines. We already shorten each long line to multiple lines. And also, we split this long function into three different functions.
<code>
<source lang="ruby">
def assign_reviewers_for_team(assignment_id,student_review_num,participants_hash,exact_num_of_review_needed)
def assign_reviewers_for_team(assignment_id,student_review_num,participants_hash,exact_num_of_review_needed)
     if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", \
     if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", \
Line 52: Line 52:
                                               last.created_at
                                               last.created_at
   end
   end
</code>
</source>


==Test==
==Test==

Revision as of 02:42, 29 October 2016

Background

Current Implementation and Problems

  1. Method names for dealing with the 5 different kinds of objects should be as similar as possible, and they should share helper functions when possible.
  2. Method response_report has some sql - like code. Rewrite with Active Record.
  3. Test whether method add_user_to_assignment is used. There is no way that this method should be in ReviewMappingController.--plz remove this method and caller.
  4. There was a self-review feature, the method add_self_reviewr, get_team_from_submission are related to this. Two views calls add_self_reviewer are ç. The names of views are not related to self_review feature. Plus those two views are not called anywhere.
  5. Method delete_all_reviewers actually only deletes the outstanding review response maps (the ones which has been initiated, but there is no response yet). So it should better be named delete_outstanding_reviewers. You can try to test this method by clicking “Assign reviewers” icon on an assignment.
  6. Method release_reservation should be renamed as release_mapping. In addition, delete it if you find this method is not called anywhere.
  7. Method delete_mappings is problematic. It does not looks like a controller method. Please refactor it or delete it if you can validate that this method is not called anywhere.
  8. Method automatic_review_mapping_strategy is too long. Please refactor and test it.

Changes Implemented

  4. By searching the whole project and routes, we verify that methods add_self_reviewr and get_team_from_submission in this controller are not called by any other methods except for views add_self_reviewr, get_team_from_submission. And those two views are not linked to any other views. So they are deleted from the project.
  5. We already renamed method delete_all_reviewers to delete_outstanding_reviewers and changed the corresponding button name at the corresponding view (_list_review_mappings.html.erb).
  6. By searching the whole project and routes, we verify that methods release_reservation in this controller are not used in anywhere.
  7. By searching the whole project and routes, we verify that methods delete_mappings in this controller are not used in anywhere.
  8. This function have many long lines. We already shorten each long line to multiple lines. And also, we split this long function into three different functions.
def assign_reviewers_for_team(assignment_id,student_review_num,participants_hash,exact_num_of_review_needed)
    if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", \
                                assignment_id, \
                                @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
      participants_with_insufficient_review_num = []
      participants_hash.each do |participant_id, review_num|
        participants_with_insufficient_review_num << participant_id if review_num < student_review_num
      end
      unsorted_teams_hash = {}
      ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", \
                               assignment_id, 0]).each do |response_map|
        unsorted_teams_hash[response_map.reviewee_id] = \
          calculate_unsorted_teams_hash_reviewee_id(unsorted_teams_hash, response_map)
      end
      teams_hash = unsorted_teams_hash.sort_by {|_, v| v }.to_h
      participants_with_insufficient_review_num.each do |participant_id|
        teams_hash.each do |team_id, _num_review_received|
          next if TeamsUser.exists?(team_id: team_id, \
                                    user_id: Participant.find(participant_id).user_id)
          ReviewResponseMap.where(reviewee_id: team_id, reviewer_id: participant_id, \
                                  reviewed_object_id: assignment_id).first_or_create
          teams_hash[team_id] += 1
          teams_hash = teams_hash.sort_by {|_, v| v }.to_h
          break
        end
      end
    end
    @@time_create_last_review_mapping_record = ReviewResponseMap.\
                                               where(reviewed_object_id: assignment_id).\
                                               last.created_at
  end

Test

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2016/E1641._Refactor_review_mapping_controller.rb&oldid=103699"