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 cognitive complexity of 6 and ABC size complexity 25. 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