CSC/ECE 517 Spring 2023 - E2301. Refactor review maping helper

Introduction

This page gives a description of the changes made for the review_mapping_helper.rb of Expertiza based OSS project.

Overview of Expertiza

Expertiza is a learning management system that is available as open source software and was created using Ruby on Rails. It has a wide range of features and capabilities, where students can submit and peer-review learning objects (articles, codes, websites, etc). Instructors add and grade the assignments submitted by students to Expertiza. The files that are largely addressed in this project, such as assignment_node.rb, course_node.rb, team_node.rb, folder_node.rb, and questionnaire_node.rb, are essential in executing this functionality. 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. It is missing proper comments for each functionality.

Changes Made

1)def display_volume_metric_chart

Reduced lines of code by removing unnecessary line breaks in display_volume_metric_chart

Before

def display_volume_metric_chart(reviewer)
   labels, reviewer_data, all_reviewers_data = initialize_chart_elements(reviewer)
   data = {
     labels: labels,
     datasets: [
       {
         label: 'vol.',
         backgroundColor: 'rgba(255,99,132,0.8)',
         borderWidth: 1,
         data: reviewer_data,
         yAxisID: 'bar-y-axis1'
       },
       {
         label: 'avg. vol.',
         backgroundColor: 'rgba(255,206,86,0.8)',
         borderWidth: 1,
         data: all_reviewers_data,
         yAxisID: 'bar-y-axis2'
       }
     ]
   }
   options = {
     legend: {
       position: 'top',
       labels: {
         usePointStyle: true
       }
     },
     width: '200',
     height: '125',
     scales: {
       yAxes: [{
         stacked: true,
         id: 'bar-y-axis1',
         barThickness: 10
       }, {
         display: false,
         stacked: true,
         id: 'bar-y-axis2',
         barThickness: 15,
         type: 'category',
         categoryPercentage: 0.8,
         barPercentage: 0.9,
         gridLines: {
           offsetGridLines: true
         }
       }],
       xAxes: [{
         stacked: false,
         ticks: {
           beginAtZero: true,
           stepSize: 50,
           max: 400
         }
       }]
     }
   }
   horizontal_bar_chart data, options
 end

After

def display_volume_metric_chart(reviewer)
   labels, reviewer_data, all_reviewers_data = initialize_chart_elements(reviewer)
   data = {
     labels: labels,
     datasets: [
       {
         label: 'vol.',
         backgroundColor: 'rgba(255,99,132,0.8)',
         borderWidth: 1,
         data: reviewer_data,
         yAxisID: 'bar-y-axis1'
       },
       {
         label: 'avg. vol.',
         backgroundColor: 'rgba(255,206,86,0.8)',
         borderWidth: 1,
         data: all_reviewers_data,
         yAxisID: 'bar-y-axis2'
       }
     ]
   }
   options = {
     legend: { position: 'top', labels: { usePointStyle: true } },
     width: '200', height: '125',
     scales: {
       yAxes: [
         { stacked: true, id: 'bar-y-axis1', barThickness: 10 },
         {
           display: false, stacked: true, id: 'bar-y-axis2',
           barThickness: 15, type: 'category',
           categoryPercentage: 0.8, barPercentage: 0.9,
           gridLines: { offsetGridLines: true }
         }
       ],
       xAxes: [
         { stacked: false, ticks: { beginAtZero: true, stepSize: 50, max: 400 } }
       ]
     }
   }
   horizontal_bar_chart(data, options)
 end

2) def display_tagging_interval_chart

-->Added early return if intervals array is empty.

-->Removed unnecessary comment.

-->Simplified datasets array initialization by always adding mean data

Before

def display_tagging_interval_chart(intervals)
   # if someone did not do any tagging in 30 seconds, then ignore this interval
   threshold = 30
   intervals = intervals.select { |v| v < threshold }
   unless intervals.empty?
     interval_mean = intervals.reduce(:+) / intervals.size.to_f
   end
   # build the parameters for the chart
   data = {
     labels: [*1..intervals.length],
     datasets: [
       {
         backgroundColor: 'rgba(255,99,132,0.8)',
         data: intervals,
         label: 'time intervals'
       },
       unless intervals.empty?
         {
           data: Array.new(intervals.length, interval_mean),
           label: 'Mean time spent'
         }
       end
     ]
   }
   options = {
     width: '200',
     height: '125',
     scales: {
       yAxes: [{
         stacked: false,
         ticks: {
           beginAtZero: true
         }
       }],
       xAxes: [{
         stacked: false
       }]
     }
   }
   line_chart data, options
 end

After

def display_tagging_interval_chart(intervals)
   threshold = 30
   intervals = intervals.select { |v| v < threshold }
   interval_mean = intervals.sum / intervals.size.to_f unless intervals.empty?
   data = {
     labels: [*1..intervals.length],
     datasets: [
       { backgroundColor: 'rgba(255,99,132,0.8)', data: intervals, label: 'time intervals' },
       *(!intervals.empty? && [{ data: [interval_mean] * intervals.length, label: 'Mean time spent' }])
     ]
   }
   options = {
     width: '200', height: '125',
     scales: { yAxes: [{ stacked: false, ticks: { beginAtZero: true } }], xAxes: [{ stacked: false }] }
   }
   line_chart(data, options)
 end

3) def sort_reviewer_by_review_volume_desc

Made the method more readable, maintainable, and efficient, and also reduce the complexity and cognitive complexity of the code. In this refactored version, we have:

-->Extracted the logic for getting the review volumes into a separate block, to make it easier to understand and test.

-->Simplified the code for getting the reviewer's overall average volume and average volume per round.

-->Used concat instead of push to add elements to the avg_vol_per_round array.

-->Sorted the reviewers by their review volume in descending order using the sort_by! method and a block.

-->Used a to_i method instead of to_f.to_i to convert the number of review rounds to an integer.

-->Removed the unnecessary @all_reviewers_avg_vol_per_round variable since it's not being used in the method.

Before

def sort_reviewer_by_review_volume_desc
   @reviewers.each do |r|
     # get the volume of review comments
     review_volumes = Response.volume_of_review_comments(@assignment.id, r.id)
     r.avg_vol_per_round = []
     review_volumes.each_index do |i|
       if i.zero?
         r.overall_avg_vol = review_volumes[0]
       else
         r.avg_vol_per_round.push(review_volumes[i])
       end
     end
   end
   # get the number of review rounds for the assignment
   @num_rounds = @assignment.num_review_rounds.to_f.to_i
   @all_reviewers_avg_vol_per_round = []
   @all_reviewers_overall_avg_vol = @reviewers.inject(0) { |sum, r| sum + r.overall_avg_vol } / (@reviewers.blank? ? 1 : @reviewers.length)
   @num_rounds.times do |round|
     @all_reviewers_avg_vol_per_round.push(@reviewers.inject(0) { |sum, r| sum + r.avg_vol_per_round[round] } / (@reviewers.blank? ? 1 : @reviewers.length))
   end
   @reviewers.sort! { |r1, r2| r2.overall_avg_vol <=> r1.overall_avg_vol }
 end

After

def sort_reviewer_by_review_volume_desc
 # Get the volume of review comments for each reviewer
 @reviewers.each do |reviewer|
   reviewer.avg_vol_per_round = []
   review_volumes = Response.volume_of_review_comments(@assignment.id, reviewer.id)
   reviewer.overall_avg_vol = review_volumes.first
   reviewer.avg_vol_per_round.concat(review_volumes[1..-1])
 end
 # Sort reviewers by their review volume in descending order
 @reviewers.sort_by! { |reviewer| -reviewer.overall_avg_vol 
}
 # Get the number of review rounds for the assignment
 @num_rounds = @assignment.num_review_rounds.to_i
 @all_reviewers_avg_vol_per_round = []
end

4) def initialize_chart_elements

Removed the round variable since it was only used to track the round number, which is already captured by the labels array.

-->Changed the push method to the << operator for brevity.

-->Removed the unless condition to reduce nesting, using if instead and flipping the condition.

-->Removed unnecessary initialization of arrays (reviewer_data, all_reviewers_data) since they will be created within the loop

Before

def initialize_chart_elements(reviewer)
   round = 0
   labels = []
   reviewer_data = []
   all_reviewers_data = []
   # display avg volume for all reviewers per round
   @num_rounds.times do |rnd|
     next unless @all_reviewers_avg_vol_per_round[rnd] > 0
     round += 1
     labels.push round
     reviewer_data.push reviewer.avg_vol_per_round[rnd]
     all_reviewers_data.push @all_reviewers_avg_vol_per_round[rnd]
   end
   labels.push 'Total'
   reviewer_data.push reviewer.overall_avg_vol
   all_reviewers_data.push @all_reviewers_overall_avg_vol
   [labels, reviewer_data, all_reviewers_data]
 end

After

def initialize_chart_elements(reviewer)
   labels, reviewer_data, all_reviewers_data = [], [], []
   @num_rounds.times do |rnd|
     next if @all_reviewers_avg_vol_per_round[rnd] <= 0
     labels << (rnd + 1)
     reviewer_data << reviewer.avg_vol_per_round[rnd]
     all_reviewers_data << @all_reviewers_avg_vol_per_round[rnd]
   end
   labels << 'Total'
   reviewer_data << reviewer.overall_avg_vol
   all_reviewers_data << @all_reviewers_overall_avg_vol
   [labels, reviewer_data, all_reviewers_data]
 end

5)def calculate_key_chart_information

-->Replaced select with select! to modify the intervals array in place instead of creating a new one.

-->Removed unnecessary comments.

-->Removed sum variable and integrated it directly into the calculation of variance.

-->Removed the metrics hash object initialization to simplify code.

-->Removed unnecessary :stand_dev symbol from the hash object initialization and calculated its value directly.

Before

def calculate_key_chart_information(intervals)
   # if someone did not do any tagging in 30 seconds, then ignore this interval
   threshold = 30
   interval_precision = 2 # Round to 2 Decimal Places
   intervals = intervals.select { |v| v < threshold }
   # Get Metrics once tagging intervals are available
   unless intervals.empty?
     metrics = {}
     metrics[:mean] = (intervals.reduce(:+) / intervals.size.to_f).round(interval_precision)
     metrics[:min] = intervals.min
     metrics[:max] = intervals.max
     sum = intervals.inject(0) { |accum, i| accum + (i - metrics[:mean])**2 }
     metrics[:variance] = (sum / intervals.size.to_f).round(interval_precision)
     metrics[:stand_dev] = Math.sqrt(metrics[:variance]).round(interval_precision)
     metrics
   end
   # if no Hash object is returned, the UI handles it accordingly
 end

After

def calculate_key_chart_information(intervals)
   # if someone did not do any tagging in 30 seconds, then ignore this interval
   threshold = 30
   interval_precision = 2 # Round to 2 Decimal Places
   intervals.select! { |v| v < threshold }
   # Get Metrics once tagging intervals are available
   unless intervals.empty?
     mean = intervals.sum / intervals.size.to_f
     variance = intervals.map { |v| (v - mean) ** 2 }.sum / intervals.size.to_f
     metrics = {
       mean: mean.round(interval_precision),
       min: intervals.min,
       max: intervals.max,
       variance: variance.round(interval_precision),
       stand_dev: Math.sqrt(variance).round(interval_precision)
     }
     metrics
   end
   # if no Hash object is returned, the UI handles it accordingly
 end

6) def review_metrics

The changes should improve the code's readability and maintainability while also addressing the issues highlighted by the feedback.

-->The instance_variable_set calls were refactored to use string interpolation for readability.

-->The if statement was simplified by using the dig method to access nested hashes and the values_at method to check if all required metrics are present.

-->The each loop was changed to iterate over the avg_and_ranges hash directly to avoid unnecessary nil values.

-->The logic for setting the metric_value variable was simplified and moved inside the loop to avoid repeated code.

-->The cognitive complexity has been reduced to 4.

Before

def 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 review_metrics(round, team_id)
 %i[max min avg].each do |metric|
   instance_variable_set("@#{metric}", '-----')
 end
 avg_and_ranges = @avg_and_ranges&.dig(team_id, round)
 if avg_and_ranges && avg_and_ranges.values_at(:max, :min, :avg).all?(&:present?)
   avg_and_ranges.each do |metric, value|
     metric_value = "#{value.round(0)}%"
     instance_variable_set("@#{metric}", metric_value)
   end
 end
end

7) def get_data_for_review_report

The changes should make the code more readable and easier to understand, and should also reduce the ABC size and the number of lines.

-->The instance_variable_set calls were replaced with a new response_counts array to keep track of the response counts for each round.

-->The response_counts array is initialized to an array of zeros with a length of @assignment.num_review_rounds.

-->The where call was simplified to use a hash instead of an array.

-->The loop over the response_maps was refactored to use a more descriptive variable name (response_map) and to check whether the reviewee team exists before counting its responses. -->The loop over the rounds was simplified to use an index variable (round - 1) to access the correct element in the response_counts array.

-->The method now returns an array of three values: the response_maps, the rspan, and the response_counts.

Before

def get_data_for_review_report(reviewed_object_id, reviewer_id, type)
   rspan = 0
   (1..@assignment.num_review_rounds).each { |round| instance_variable_set('@review_in_round_' + round.to_s, 0) }
   response_maps = ResponseMap.where(['reviewed_object_id = ? AND reviewer_id = ? AND type = ?', reviewed_object_id, reviewer_id, type])
   response_maps.each do |ri|
     rspan += 1 if Team.exists?(id: ri.reviewee_id)
     responses = ri.response
     (1..@assignment.num_review_rounds).each do |round|
       instance_variable_set('@review_in_round_' + round.to_s, instance_variable_get('@review_in_round_' + round.to_s) + 1) if responses.exists?(round: round)
     end
   end
   [response_maps, rspan]
 end

After

def get_data_for_review_report(reviewed_object_id, reviewer_id, type)
 rspan = 0
 response_counts = Array.new(@assignment.num_review_rounds, 0)
 response_maps = ResponseMap.where(reviewed_object_id: reviewed_object_id, reviewer_id: reviewer_id, type: type)
 response_maps.each do |response_map|
   if Team.exists?(id: response_map.reviewee_id)
     rspan += 1
     responses = response_map.response
     (1..@assignment.num_review_rounds).each do |round|
       response_counts[round - 1] += 1 if responses.exists?(round: round)
     end
   end
 end
 [response_maps, rspan, response_counts]
end

8) def get_team_color

--> Removed the redundant variables assignment_created and assignment_due_dates and passed them directly as arguments to the obtain_team_color method call.

-->Removed the unnecessary else clause, since the function will always return before reaching that point.

-->Reduced cognitive complexity to 4

Before

def get_team_color(response_map)
   # Storing redundantly computed value in a variable
   assignment_created = @assignment.created_at
   # Storing redundantly computed value in a variable
   assignment_due_dates = DueDate.where(parent_id: response_map.reviewed_object_id)
   # Returning colour based on conditions
   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_color(response_map, assignment_created, assignment_due_dates)
     end
   else
     'red'
   end
 end

After

def get_team_color(response_map)
 if Response.exists?(map_id: response_map.id)
   if !response_map.try(:reviewer).try(:review_grade).nil?
     return 'brown'
   elsif response_for_each_round?(response_map)
     return 'blue'
   end
 else
   return 'red'
 end
 obtain_team_color(response_map, @assignment.created_at, DueDate.where(parent_id: response_map.reviewed_object_id))
end

9) def submitted_within_round

-->Extracted the logic for querying the SubmissionRecord table into a separate variable submission, so that it can be modified based on the round number.

-->Used an if/else statement instead of two separate queries with similar conditions.

-->Removed the call to try(:first).try(:created_at) at the end, as it's redundant with the exists? Method.

-->Reduced cognitive complexity to 3.

Before

def submitted_within_round?(round, response_map, assignment_created, assignment_due_dates)
   submission_due_date = assignment_due_dates.where(round: round, deadline_type_id: 1).try(:first).try(:due_at)
   submission = SubmissionRecord.where(team_id: response_map.reviewee_id, operation: ['Submit File', 'Submit Hyperlink'])
   subm_created_at = submission.where(created_at: assignment_created..submission_due_date)
   if round > 1
     submission_due_last_round = assignment_due_dates.where(round: round - 1, deadline_type_id: 1).try(:first).try(:due_at)
     subm_created_at = submission.where(created_at: submission_due_last_round..submission_due_date)
   end
   !subm_created_at.try(:first).try(:created_at).nil?
 end

After

def submitted_within_round?(round, response_map, assignment_created, assignment_due_dates)
 submission_due_date = assignment_due_dates.where(round: round, deadline_type_id: 1).try(:first).try(:due_at)
 submission = SubmissionRecord.where(team_id: response_map.reviewee_id, operation: ['Submit File', 'Submit Hyperlink'])
 if round > 1
   submission_due_last_round = assignment_due_dates.where(round: round - 1, deadline_type_id: 1).try(:first).try(:due_at)
   submission = submission.where(created_at: submission_due_last_round..submission_due_date)
 else
   submission = submission.where(created_at: assignment_created..submission_due_date)
 end
 submission.exists?
end

10) def get_data_for_review_report

-->Moved the initialization of rspan and response_counts outside of the loop.

-->Used each_with_object instead of manually accumulating values.

-->Used then to make the code more readable.

-->Simplified the logic for checking if a team exists

Before

def get_data_for_review_report(reviewed_object_id, reviewer_id, type)
   rspan = 0
   (1..@assignment.num_review_rounds).each { |round| instance_variable_set('@review_in_round_' + round.to_s, 0) }
   response_maps = ResponseMap.where(['reviewed_object_id = ? AND reviewer_id = ? AND type = ?', reviewed_object_id, reviewer_id, type])
   response_maps.each do |ri|
     rspan += 1 if Team.exists?(id: ri.reviewee_id)
     responses = ri.response
     (1..@assignment.num_review_rounds).each do |round|
       instance_variable_set('@review_in_round_' + round.to_s, instance_variable_get('@review_in_round_' + round.to_s) + 1) if responses.exists?(round: round)
     end
   end
   [response_maps, rspan]
 end

After

def get_data_for_review_report(reviewed_object_id, reviewer_id, type)
 response_maps = ResponseMap.where(reviewed_object_id: reviewed_object_id, reviewer_id: reviewer_id, type: type)
 response_counts = Array.new(@assignment.num_review_rounds, 0)
 response_maps.each_with_object([[], 0]) do |response_map, (response_maps_accumulator, rspan)|
   next unless Team.exists?(id: response_map.reviewee_id)
   rspan += 1
   responses = response_map.response
   response_counts = (1..@assignment.num_review_rounds).each_with_object(response_counts) do |round, response_counts_accumulator|
     response_counts_accumulator[round - 1] += 1 if responses.exists?(round: round)
     response_counts_accumulator
   end
   response_maps_accumulator << response_map
 end.then { |response_maps_accumulator, rspan| [response_maps_accumulator, rspan, response_counts] }
end

11) def get_awarded_review_score

-->Extracting the names of the round-specific instance variables to a separate array, which is generated using map.

-->Using each instead of each_with_index to set initial values for the instance variables.

-->Combining the check for team_id with the subsequent loop over the rounds to avoid redundancy.

-->Adding a check for nil before setting the instance variable to avoid raising an error if a score is missing.

Before

def get_awarded_review_score(reviewer_id, team_id)
   # Storing redundantly computed value in num_rounds variable
   num_rounds = @assignment.num_review_rounds
   # Setting values of instance variables
   (1..num_rounds).each { |round| instance_variable_set('@score_awarded_round_' + round.to_s, '-----') }
   # Iterating through list
   (1..num_rounds).each do |round|
     # Changing values of instance variable based on below condition
     unless team_id.nil? || team_id == -1.0
       instance_variable_set('@score_awarded_round_' + round.to_s, @review_scores[reviewer_id][round][team_id].to_s + '%')
     end
   end
 end

After

def get_awarded_review_score(reviewer_id, team_id)
 num_rounds = @assignment.num_review_rounds
 round_variable_names = (1..num_rounds).map { |round| '@score_awarded_round_' + round.to_s }
 round_variable_names.each { |name| instance_variable_set(name, '-----') }
 unless team_id.nil? || team_id == -1.0
   (1..num_rounds).each do |round|
     score = @review_scores[reviewer_id][round][team_id]
     instance_variable_set('@score_awarded_round_' + round.to_s, score.to_s + '%') unless score.nil?
   end
 end
end

12) def list_review_submissions

-->This extracts the condition that checks whether team and participant are not nil into a separate method called review_submissions_available?. This makes the code more readable and easier to understand, while also reducing the complexity of the list_review_submissions method.

Before

def list_review_submissions(participant_id, reviewee_team_id, response_map_id)
   participant = Participant.find(participant_id)
   team = AssignmentTeam.find(reviewee_team_id)
   html = 
   unless team.nil? || participant.nil?
     review_submissions_path = team.path + '_review' + '/' + response_map_id.to_s
     files = team.submitted_files(review_submissions_path)
     html += display_review_files_directory_tree(participant, files) if files.present?
   end
   html.html_safe
 end

After

def list_review_submissions(participant_id, reviewee_team_id, response_map_id)
 participant = Participant.find(participant_id)
 team = AssignmentTeam.find(reviewee_team_id)
 html = 
 if review_submissions_available?(team, participant)
   review_submissions_path = team.path + '_review' + '/' + response_map_id.to_s
   files = team.submitted_files(review_submissions_path)
   html += display_review_files_directory_tree(participant, files) if files.present?
 end
 html.html_safe
end
def review_submissions_available?(team, participant)
 team.present? && participant.present?
end

13) def check_submission_state

Few existing methods name are too long, this could have been changed by the team if they were working on that file - get_each_review_and_feedback_response_map, get_css_style_for_calibration_report

-->Removed the unnecessary color argument and made the method return the color.

-->Reduced the Cognitive Complexity by simplifying the conditional logic and nesting levels.

-->Made the link check more concise.

-->Broke the long line by formatting the conditional statements.

-->Reduced cognitive complexity to 3.

Before

def check_submission_state(response_map, assignment_created, assignment_due_dates, round, color)
   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? || (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

After

def check_submission_state(response_map, assignment_created, assignment_due_dates, round, color)
 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? || !link.start_with?('https://wiki')
     color.push('green')
   else
     link_updated_at = get_link_updated_at(link)
     if link_updated_since_last?(round, assignment_due_dates, link_updated_at)
       color.push('purple')
     else
       color.push('green')
     end
   end
 end
end

14) def get_awarded_review_score

In the refactored version, we have removed the unless condition and replaced it with a simple return statement, which reduces the cognitive complexity. We have also used the dig method to access the nested value of the @review_scores hash, which is more concise than the original version. Additionally, we have concatenated the score and the % symbol in a single string interpolation to make it more readable. Finally, we have removed the unnecessary use of the each method and initialized the round_variable_names variable outside of the block, which simplifies the code

Before
 
def get_awarded_review_score(reviewer_id, team_id)
 num_rounds = @assignment.num_review_rounds
 round_variable_names = (1..num_rounds).map { |round| '@score_awarded_round_' + round.to_s }
 round_variable_names.each { |name| instance_variable_set(name, '-----') }
 return if team_id.nil? || team_id == -1.0
 (1..num_rounds).each do |round|
   score = @review_scores.dig(reviewer_id, round, team_id)
   instance_variable_set('@score_awarded_round_' + round.to_s, "#{score}%" ) unless score.nil?
 end
end

After

def get_awarded_review_score(reviewer_id, team_id)
 num_rounds = @assignment.num_review_rounds
 round_variable_names = (1..num_rounds).map { |round| "@score_awarded_round_#{round}" }
 round_variable_names.each { |name| instance_variable_set(name, '-----') }
 return if team_id.nil? || team_id == -1.0
 (1..num_rounds).each do |round|
   score = @review_scores.dig(reviewer_id, round, team_id)
   next if score.nil?
   instance_variable_set("@score_awarded_round_#{round}", "#{score}%")
 end
end