CSC/ECE 517 Fall 2019 - E1948. Refactor review mapping helper.rb
Introduction
This page gives a description of the changes made for the review_mapping_helper.rb of Expertiza based OSS project.
Expertiza Background
Expertiza is a web application where students can submit and peer-review learning objects (articles, codes, websites, etc). Instructors add and grade the assignments submitted by students to Expertiza. Students can be assigned in teams based on their selection of the topics. It has functionalities such as peer reviews in which students can provide feedback on other's work which helps peer in better developing the project. It is supported by the National Science Foundation.
Problem Statement
The review_mapping_helper.rb has multiple functions with a high complexities namely - Cognitive, Perceived, Cyclomatic, Assignment Branch Condition size (ABC size) and Lines of Code (LOC). The review_mapping_helper.rb has methods which exceeds the limit on lines of code Also, it is missing proper comments for each functionality. Cyclomatic complexity of most of the methods is way too high as per the standard defined in the Code Climate.
Flowchart
The following process is carried out to complete the project-
Refactoring
The method get_review_metrics had cognitive complexity of 6 and ABC size complexity 25. It was refactored using .dig() function and removing .to_s function by changing the variable type.
Before:
def get_review_metrics(round, team_id) %i[max min avg].each {|metric| instance_variable_set('@' + metric.to_s, '-----') } if @avg_and_ranges[team_id] && @avg_and_ranges[team_id][round] && %i[max min avg].all? {|k| @avg_and_ranges[team_id][round].key? k } %i[max min avg].each do |metric| metric_value = @avg_and_ranges[team_id][round][metric].nil? ? '-----' : @avg_and_ranges[team_id][round][metric].round(0).to_s + '%' instance_variable_set('@' + metric.to_s, metric_value) end end end
After:
def get_review_metrics(round, team_id) ['max', 'min', 'avg'].each {|metric| instance_variable_set('@' + metric, '-----') } x = @avg_and_ranges.dig(team_id, round) if x && %i[max min avg].all? {|k| x.key? k } ['max', 'min', 'avg'].each do |metric| average_metric = @avg_and_ranges.dig(team_id, round, metric) metric_value = average_metric.nil? ? '-----' : average_metric.round(0).to_s + '%' instance_variable_set('@' + metric, metric_value) end end end
The method get_awarded_review_score had ABC size complexity of 19. It was refactored using .dig() function and creating new variable for redundant computations.
Before:
def get_awarded_review_score(reviewer_id, team_id) (1..@assignment.num_review_rounds).each {|round| instance_variable_set("@score_awarded_round_" + round.to_s, '-----') } (1..@assignment.num_review_rounds).each do |round| if @review_scores[reviewer_id] && @review_scores[reviewer_id][round] && @review_scores[reviewer_id][round][team_id] && @review_scores[reviewer_id][round][team_id] != -1.0 instance_variable_set("@score_awarded_round_" + round.to_s, @review_scores[reviewer_id][round][team_id].inspect + '%') end end end
After:
def get_awarded_review_score(reviewer_id, team_id) num_rounds = @assignment.num_review_rounds (1..num_rounds).each {|round| instance_variable_set("@score_awarded_round_" + round.to_s, '-----') } (1..num_rounds).each do |round| teamID = @review_scores.dig(reviewer_id, round, team_id) if teamID != nil && teamID != -1.0 instance_variable_set("@score_awarded_round_" + round.to_s, teamID.inspect + '%') end end end
The method get_team_color had ABC size complexity of 19. It was refactored using .dig() function and creating new variable for redundant computations.
Before:
def get_team_colour(response_map) assignment_created = @assignment.created_at assignment_due_dates = DueDate.where(parent_id: response_map.reviewed_object_id) if Response.exists?(map_id: response_map.id) if !response_map.try(:reviewer).try(:review_grade).nil? 'brown' elsif response_for_each_round?(response_map) 'blue' else color = [] (1..@assignment.num_review_rounds).each do |round| if submitted_within_round?(round, response_map, assignment_created, assignment_due_dates) color.push 'purple' else link = submitted_hyperlink(round, response_map, assignment_created, assignment_due_dates) if link.nil? or (link !~ %r{https*:\/\/wiki(.*)}) # can be extended for github links in future color.push 'green' else link_updated_at = get_link_updated_at(link) color.push link_updated_since_last?(round, assignment_due_dates, link_updated_at) ? 'purple' : 'green' end end end color[-1] end else 'red' end end
After:
def get_team_colour(response_map) assignment_created = @assignment.created_at assignment_due_dates = DueDate.where(parent_id: response_map.reviewed_object_id) if Response.exists?(map_id: response_map.id) if !response_map.try(:reviewer).try(:review_grade).nil? 'brown' elsif response_for_each_round?(response_map) 'blue' else obtain_team_colour(response_map,assignment_created,assignment_due_dates) end else 'red' end end # loops through the number of assignment review rounds and obains the team colour def obtain_team_colour(response_map, assignment_created, assignment_due_dates) color = [] (1..@assignment.num_review_rounds).each do |round| check_submission_state(response_map, assignment_created, assignment_due_dates, round) end color[-1] end # checks the submission state within each round and assigns team colour def check_submission_state(response_map, assignment_created, assignment_due_dates, round) if submitted_within_round?(round, response_map, assignment_created, assignment_due_dates) color.push 'purple' else link = submitted_hyperlink(round, response_map, assignment_created, assignment_due_dates) if link.nil? or (link !~ %r{https*:\/\/wiki(.*)}) # can be extended for github links in future color.push 'green' else link_updated_at = get_link_updated_at(link) color.push link_updated_since_last?(round, assignment_due_dates, link_updated_at) ? 'purple' : 'green' end end end
The method get_each_review_and_feedback_map had ABC size complexity of 19. It was refactored using .dig() function and creating new variable for redundant computations.
Before:
def get_each_review_and_feedback_response_map(author) @team_id = TeamsUser.team_id(@id.to_i, author.user_id) # Calculate how many responses one team received from each round # It is the feedback number each team member should make @review_response_map_ids = ReviewResponseMap.where(["reviewed_object_id = ? and reviewee_id = ?", @id, @team_id]).pluck("id") {1 => 'one', 2 => 'two', 3 => 'three'}.each do |key, round_num| instance_variable_set('@review_responses_round_' + round_num, Response.where(["map_id IN (?) and round = ?", @review_response_map_ids, key])) # Calculate feedback response map records instance_variable_set('@feedback_response_maps_round_' + round_num, FeedbackResponseMap.where(["reviewed_object_id IN (?) and reviewer_id = ?", instance_variable_get('@all_review_response_ids_round_' + round_num), author.id])) end # rspan means the all peer reviews one student received, including unfinished one @rspan_round_one = @review_responses_round_one.length @rspan_round_two = @review_responses_round_two.length @rspan_round_three = @review_responses_round_three.nil? ? 0 : @review_responses_round_three.length end
After:
def get_each_review_and_feedback_response_map(author) @team_id = TeamsUser.team_id(@id.to_i, author.user_id) # Calculate how many responses one team received from each round # It is the feedback number each team member should make @review_response_map_ids = ReviewResponseMap.where(["reviewed_object_id = ? and reviewee_id = ?", @id, @team_id]).pluck("id") feedback_response_map_record(author) # rspan means the all peer reviews one student received, including unfinished one @rspan_round_one = @review_responses_round_one.length @rspan_round_two = @review_responses_round_two.length @rspan_round_three = @review_responses_round_three.nil? ? 0 : @review_responses_round_three.length end def feedback_response_map_record(author) {1 => 'one', 2 => 'two', 3 => 'three'}.each do |key, round_num| instance_variable_set('@review_responses_round_' + round_num, Response.where(["map_id IN (?) and round = ?", @review_response_map_ids, key])) # Calculate feedback response map records instance_variable_set('@feedback_response_maps_round_' + round_num, FeedbackResponseMap.where(["reviewed_object_id IN (?) and reviewer_id = ?", instance_variable_get('@all_review_response_ids_round_' + round_num), author.id])) end end
The method get_css_style_for_calibration_report had ABC size complexity of 19. It was refactored using .dig() function and creating new variable for redundant computations.
Before:
def get_css_style_for_calibration_report(diff) # diff - difference between stu's answer and instructor's answer dict = {0 => 'c5',1 => 'c4',2 => 'c3',3 => 'c2'} if dict.key?(diff.abs) css_class = dict[diff.abs] else css_class = 'c1' end css_class end
After:
def get_css_style_for_calibration_report(diff) # diff - difference between stu's answer and instructor's answer dict = {0 => 'c5', 1 => 'c4', 2 => 'c3', 3 => 'c2'} css_class = if dict.key?(diff.abs) dict[diff.abs] else css_class = 'c1' end css_class end