CSC/ECE 517 Fall 2022 - E2255. Refactor review mapping helper.rb

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

About Expertiza

Expertiza is an open-source project based on the 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 of Expertise to work on various projects and assignments. Students can also peer review other students' submissions. Expertise supports submission across various document types, including URLs and wiki pages.

Problem Statement

The following tasks were accomplished in this project:

• Use Code Climate to do a diagnosis for Expertiza. Search for the issues involving the files mentioned above.
• Fix all the methods having the issue with cyclomatic complexity, assignment branch condition size, extra commas, space or lack of it, and assigning branching condition.
• Write test cases of any new code written. Test the existing functionality after refactoring

Problems and Solutions

Problem 1

• Use Code Climate to do a diagnosis for Expertiza. Search for the issues involving the files mentioned above.

Example:

Problem 2

• Fix all the methods having the issue with cyclomatic complexity, assignment branch condition size, extra commas, space or lack of it, and assigning branching condition.

Example One: Cyclomatic complexity Error, check_submission_state has Cyclomatic complexity 10 Maximum 5

 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

Solution: The problem is caused by nesting loops so we break it up

  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'
      return
    end

    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'
      return
    end

    link_updated_at = get_link_updated_at(link)
    color.push link_updated_since_last?(round, assignment_due_dates, link_updated_at) ? 'purple' : 'green'
  end

Example Two: Inconsistent Capitalization on comments

  # loops through the number of assignment review rounds and obtains the team colour
  def obtain_team_color(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, color)
    end

    color[-1]
  end

Solution: Since many comments have inconsistent capitalization, our group went over the code line by line and manually fixed all inconsistent capitalizations.

  # Loops through the number of assignment review rounds and obtains the team colour
  def obtain_team_color(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, color)
    end

    color[-1]
  end

Example Three: Inconsistent Spacing

  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'])
    submission_due_date = assignment_due_dates.where(round: round, deadline_type_id: 1).try(:first).try(:due_at)
    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

Solution: Many functions within the file are clustered like the example above, we have to go through the code line by line and add spacing before lines to make the code more readable

 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'])

    submission_due_date = assignment_due_dates.where(round: round, deadline_type_id: 1).try(:first).try(:due_at)
    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

Example Four: Add comments for some functions.

  #
  # for calibration report
  #
  def get_css_style_for_calibration_report(diff)
    
    dict = { 0 => 'c5', 1 => 'c4', 2 => 'c3', 3 => 'c2' }

    if dict.key?(diff.abs)
      dict[diff.abs]
    else
      'c1'
    end
  end

Solution: many of the functions inside the code have an inconsistent format and lack comments, our team went through the code line by line to edit and add comments.

  # Gets CSS style for calibration report
  def get_css_style_for_calibration_report(diff)
  # Diff - the difference between stu's answer and instructor's answer
  dict = { 0 => 'c5', 1 => 'c4', 2 => 'c3', 3 => 'c2' }
  
  if dict.key?(diff.abs)
      dict[diff.abs]
    else
      'c1'
    end
  end

Problem 3

• Write test cases of any new code written. Test the existing functionality after refactoring

Note: Due to the majority of the code already being refactored, Thus our team did not make any major changes to the review_mapping_helper.rb file. We did however use RSPEC to test all existing functions.

$ rspec spec/helpers/review_mapping_helper_spec.rb
$ rspec spec/features/review_mapping_helper_spec.rb

Work Flow Diagram

Travis-CI

RSPEC Testing

NOTE: there are existing errors already inside the code before we started refactoring. And after refactoring, Our changes did not regress any functionality. Since our responsibility is only refactoring the code and not fixing the functionalities, the RSPEC Testing result showed that we have done our due diligence.

Result RSPEC testing Before Refactoring

Command Used:

rspec spec/helpers/review_mapping_helper_spec.rb

result:

61 examples, 0 failures - spec

Command Used:

rspec spec/features/review_mapping_helper_spec.rb

21 examples, 20 failures - features

Result Rspec testing After Refactoring:

Our change did not regress any functionality

Admin Account Information

Username: instructor6

Password: password

Project Mentor

Edward Gehringer (efg@ncsu.edu)

Jialin Cui (jcui9@ncsu.edu)

Team Member

Zanxiang Wang (zwang236@ncsu.edu)

Nikhil Mehra (nmehra2@ncsu.edu)

Brian Davis (bbdavis4@ncsu.edu)

Reference

1. VCL link
2. Github Repo
3. Github Pull Request
4. Expertiza on GitHub
5. GitHub Project Repository Fork
6. Expertiza Project Documentation Wiki
7. CodeClimate Documentation