CSC/ECE 517 Spring 2020 - Project E2008. Refactor summary helper.rb

About Expertiza

Expertiza is a web application developed using Ruby on Rails Framework whose creation and maintenance are taken care of by students as well as the faculty of NCSU. Its code is available on Github Expertiza on GitHub. Expertiza allows the instructor to create and edit new as well as existing assignments. This also includes creating a wide range of topics under each assignment that students can sign up for. They can also publish surveys and reviews, view statistical results, set deadlines for assignments and make announcements. It provides a platform for students to signup for topics, form teams, view and submit assignments and give peer reviews and feedback.

Problem Statement

Background: In Expertiza students can review each other’s projects and even each other as teammates. Students can view their project scores and instructors can view student's teammate review scores on the view scores page. This Summary helper aids in calculating these scores and rendering the results on the view scores section of an assignment. Summary helper is a helper module that consists of methods used to calculate scores for these reviews. This is for the use of instructors.

Requirement: E2008 is an Expertiza OSS project which deals basically with refactoring app/helpers/summary_helper.rb to reduce the code climate issues such as

Assignment Branch Condition
Cognitive Complexity
Method/line too long
Unused variables / access modifiers

A good method should have:

Assignment Branch Condition size <= 15
Cognitive Complexity <= 6
No. of lines in a method <= 25

Issues found

The following are issues which were found in the code:

Assignment Branch Condition

Method	ABC Size
summarize_reviews_by_reviewee	44.61
summarize_reviews_by_criterion	42.34
summarize_reviews_by_reviewees	89.93
get_questions_by_assignment	16.43
calculate_avg_score_by_round	18.57

Cognitive Complexity

Method	Cognitive Complexity
summarize_reviews_by_criterion	16
summarize_reviews_by_reviewees	17
get_questions_by_assignment	11
break_up_comments_to_sentences	6

Method/line too long

Method/Line too long	Comments
Method: summarize_reviews_by_reviewees	Lines of Code:39
Method: summarize_reviews_by_criterion	Lines of Code:27
Line: 145	self.avg_scores_by_round[reviewee.name][round] too long (162/160)

Unused variables / access modifiers

Vaiable/Access Modifier	Comments
Variable: summary	Method: summarize_sentences
Variable: included_question_counter	Method: summarize_reviews_by_reviewee
Access Modifier: module_function	Class: Summary

Solution Implemented

Refactor - summarize_reviews_by_reviewee

This method is used to summarize reviews by a reviewer for each question.

Changes Made:

This method could be refactored into smaller methods namely summarize_reviews_by_reviewee and summarize_reviews_by_reviewee_assign where summarize_reviews_by_reviewee calls summarize_reviews_by_reviewee_assign to get average scores and summary for each question.

Before:

    def summarize_reviews_by_reviewee(questions, assignment, r_id, summary_ws_url)
      self.summary = ({})
      self.avg_scores_by_round = ({})
      self.avg_scores_by_criterion = ({})

      # get all answers for each question and send them to summarization WS
      questions.keys.each do |round|
        self.summary[round.to_s] = {}
        self.avg_scores_by_criterion[round.to_s] = {}
        self.avg_scores_by_round[round.to_s] = 0.0
        included_question_counter = 0

        questions[round].each do |q|
          next if q.type.eql?("SectionHeader")

          self.summary[round.to_s][q.txt] = ""
          self.avg_scores_by_criterion[round.to_s][q.txt] = 0.0

          question_answers = Answer.answers_by_question_for_reviewee(assignment.id, r_id, q.id)

          max_score = get_max_score_for_question(q)

          comments = break_up_comments_to_sentences(question_answers)

          # get the avg scores for this question
          self.avg_scores_by_criterion[round.to_s][q.txt] = calculate_avg_score_by_criterion(question_answers, max_score)
          # get the summary of answers to this question
          self.summary[round.to_s][q.txt] = summarize_sentences(comments, summary_ws_url)
        end
        self.avg_scores_by_round[round.to_s] = calculate_avg_score_by_round(self.avg_scores_by_criterion[round.to_s], questions[round])
      end
      self
    end

After:

    # produce average score and summary of comments for reviews by a reviewer for each question
    def summarize_reviews_by_reviewee(questions, assignment, reviewee_id, summary_ws_url)
      self.summary = ({})
      self.avg_scores_by_round = ({})
      self.avg_scores_by_criterion = ({})
      self.summary_ws_url = summary_ws_url

      # get all answers for each question and send them to summarization WS
      questions.each_key do |round|
        self.summary[round.to_s] = {}
        self.avg_scores_by_criterion[round.to_s] = {}
        self.avg_scores_by_round[round.to_s] = 0.0
        questions[round].each do |question|
          next if question.type.eql?("SectionHeader")
          summarize_reviews_by_reviewee_question(assignment, reviewee_id, question, round)
        end
        self.avg_scores_by_round[round.to_s] = calculate_avg_score_by_round(self.avg_scores_by_criterion[round.to_s], questions[round])
      end
      self
    end

    # get average scores and summary for each question in a review by a reviewer
    def summarize_reviews_by_reviewee_question(assignment, reviewee_id, question, round)
      question_answers = Answer.answers_by_question_for_reviewee(assignment.id, reviewee_id, question.id)

      self.avg_scores_by_criterion[round.to_s][question.txt] = calculate_avg_score_by_criterion(question_answers, get_max_score_for_question(question))

      self.summary[round.to_s][question.txt] = summarize_sentences(break_up_comments_to_sentences(question_answers), self.summary_ws_url)
    end

Impact:

Assignment Branch Condition size for summarize_reviews_by_reviewee is reduced from 44.61 to 20.64.

Refactor - summarize_reviews_by_criterion

This method is used to summarize the review for each questions

Changes Made:

This method was refactored into 3 smaller methods namely summarize_reviews_by_criterion, summarize_reviews_by_criterion_questions and end_threads.

The method summarize_reviews_by_criterion calls summarize_reviews_by_criterion_questions to get answers of each question in the rubric.

The method summarize_reviews_by_criterion_questions starts many threads to process each question and closes it by calling the function end_threads.

Before:

    # produce summaries for instructor. it merges all feedback given to all reviewees, and summarize them by criterion
    def summarize_reviews_by_criterion(assignment, summary_ws_url)
      # @summary[reviewee][round][question]
      # @avg_score_round[reviewee][round]
      # @avg_scores_by_criterion[reviewee][round][criterion]
      nround = assignment.rounds_of_reviews
      self.summary = Array.new(nround)
      self.avg_scores_by_criterion = Array.new(nround)
      self.avg_scores_by_round = Array.new(nround)
      threads = []
      rubric = get_questions_by_assignment(assignment)

      (0..nround - 1).each do |round|
        self.avg_scores_by_round[round] = 0.0
        self.summary[round] = {}
        self.avg_scores_by_criterion[round] = {}

        questions_used_in_round = rubric[assignment.varying_rubrics_by_round? ? round : 0]
        # get answers of each question in the rubric
        questions_used_in_round.each do |question|
          next if question.type.eql?("SectionHeader")
          answers_questions = Answer.answers_by_question(assignment.id, question.id)

          max_score = get_max_score_for_question(question)
          # process each question in a seperate thread
          threads << Thread.new do
            comments = break_up_comments_to_sentences(answers_questions)
            # store each avg in a hashmap and use the question as the key
            self.avg_scores_by_criterion[round][question.txt] = calculate_avg_score_by_criterion(answers_questions, max_score)
            self.summary[round][question.txt] = summarize_sentences(comments, summary_ws_url) unless comments.empty?
          end
          # Wait for all threads to end
          threads.each do |t|
            # Wait for the thread to finish if it isn't this thread (i.e. the main thread).
            t.join if t != Thread.current
          end
        end
        self.avg_scores_by_round[round] = calculate_avg_score_by_round(avg_scores_by_criterion[round], questions_used_in_round)
      end
      self
    end

After:

    # produce summaries for instructor. it merges all feedback given to all reviewees, and summarize them by criterion
    def summarize_reviews_by_criterion(assignment, summary_ws_url)
      self.summary = self.avg_scores_by_criterion = self.avg_scores_by_round = Array.new(assignment.rounds_of_reviews)
      rubric = get_questions_by_assignment(assignment)

      # get question in each round and summarize them all
      (0..assignment.rounds_of_reviews - 1).each do |round|
        questions_used_in_round = rubric[assignment.varying_rubrics_by_round? ? round : 0]
        questions_used_in_round.each do |question|
          next if question.type.eql?("SectionHeader")
          summarize_reviews_by_criterion_question(assignment, summary_ws_url, round, question)
        end
        self.avg_scores_by_round[round] = calculate_avg_score_by_round(avg_scores_by_criterion[round], questions_used_in_round)
      end
      self
    end

    # get summary of answers of each question in the rubric
    def summarize_reviews_by_criterion_question(assignment, summary_ws_url, round, question)
      threads = []
      answers_questions = Answer.answers_by_question(assignment.id, question.id)

      threads << Thread.new do
        self.avg_scores_by_criterion[round][question.txt] = calculate_avg_score_by_criterion(answers_questions, get_max_score_for_question(question))
        self.summary[round][question.txt] = summarize_sentences(break_up_comments_to_sentences(answers_questions), summary_ws_url)
      end
      # Wait for all threads to end
      end_threads(threads)
    end

    # Wait for threads to end
    def end_threads(threads)
      threads.each do |t|
        # Wait for the thread to finish if it isn't this thread (i.e. the main thread).
        t.join if t != Thread.current
      end
    end

Impact:

Assignment Branch Condition size for summarize_reviews_by_criterion is reduced from 42.34 to 17.2
Cognitive complexity is reduced from 16 to 7

Refactor - summarize_reviews_by_reviewees

This method is used to produce summaries for instructor and students. It sums up the feedback by criterion for each reviewer

Changes Made:

This method was refactored into 4 smaller methods namely summarize_reviews_by_reviewees, summarize_reviews_by_teams, summarize_by_reviewee_round and end_threads.

The method summarize_reviews_by_reviewees calls summarize_reviews_by_teams which inturn calls summarize_by_reviewee_round to get answers of each reviewer by rubric.

The method summarize_by_reviewee_round starts many threads to create requests to summarize the comments and closes the threads by calling the function end_threads.

Before:

    def summarize_reviews_by_reviewees(assignment, summary_ws_url)
      # @summary[reviewee][round][question]
      # @reviewers[team][reviewer]
      # @avg_scores_by_reviewee[team]
      # @avg_score_round[reviewee][round]
      # @avg_scores_by_criterion[reviewee][round][criterion]
      self.summary = ({})
      self.avg_scores_by_reviewee = ({})
      self.avg_scores_by_round = ({})
      self.avg_scores_by_criterion = ({})
      self.reviewers = ({})
      threads = []

      # get all criteria used in each round
      rubric = get_questions_by_assignment(assignment)

      # get all teams in this assignment
      teams = Team.select(:id, :name).where(parent_id: assignment.id).order(:name)

      teams.each do |reviewee|
        self.summary[reviewee.name] = []
        self.avg_scores_by_reviewee[reviewee.name] = 0.0
        self.avg_scores_by_round[reviewee.name] = []
        self.avg_scores_by_criterion[reviewee.name] = []

        # get the name of reviewers for display only
        self.reviewers[reviewee.name] = get_reviewers_by_reviewee_and_assignment(reviewee, assignment.id)

        # get answers of each reviewer by rubric
        (0..assignment.rounds_of_reviews - 1).each do |round|
          self.summary[reviewee.name][round] = {}
          self.avg_scores_by_round[reviewee.name][round] = 0.0
          self.avg_scores_by_criterion[reviewee.name][round] = {}

          # iterate each round and get answers
          # if use the same rubric, only use rubric[0]
          rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
          rubric_questions_used.each do |q|
            next if q.type.eql?("SectionHeader")
            summary[reviewee.name][round][q.txt] = ""
            self.avg_scores_by_criterion[reviewee.name][round][q.txt] = 0.0

            # get all answers to this question
            question_answers = Answer.answers_by_question_for_reviewee_in_round(assignment.id, reviewee.id, q.id, round + 1)
            # get max score of this rubric
            q_max_score = get_max_score_for_question(q)

            comments = break_up_comments_to_sentences(question_answers)
            # get score and summary of answers for each question
            self.avg_scores_by_criterion[reviewee.name][round][q.txt] = calculate_avg_score_by_criterion(question_answers, q_max_score)

            # summarize the comments by calling the summarization Web Service

            # since it'll do a lot of request, do this in seperate threads
            threads << Thread.new do
              summary[reviewee.name][round][q.txt] = summarize_sentences(comments, summary_ws_url) unless comments.empty?
            end
          end
          self.avg_scores_by_round[reviewee.name][round] = calculate_avg_score_by_round(self.avg_scores_by_criterion[reviewee.name][round], rubric_questions_used)
        end
        self.avg_scores_by_reviewee[reviewee.name] = calculate_avg_score_by_reviewee(self.avg_scores_by_round[reviewee.name], assignment.rounds_of_reviews)
      end

      # Wait for all threads to end
      threads.each do |t|
        t.join if t != Thread.current
      end

      self
    end

After:

    # produce summaries for instructor and students. It sum up the feedback by criterion for each reviewee
    def summarize_reviews_by_reviewees(assignment, summary_ws_url)
      self.summary = ({})
      self.avg_scores_by_reviewee = ({})
      self.avg_scores_by_round = ({})
      self.avg_scores_by_criterion = ({})
      self.reviewers = ({})
      self.summary_ws_url = summary_ws_url

      # get all criteria used in each round
      rubric = get_questions_by_assignment(assignment)

      # get all teams in this assignment
      teams = Team.select(:id, :name).where(parent_id: assignment.id).order(:name)

      teams.each do |reviewee|
        summarize_reviews_by_team_reviewee(assignment, reviewee, rubric)
        self.avg_scores_by_reviewee[reviewee.name] = calculate_avg_score_by_reviewee(self.avg_scores_by_round[reviewee.name], assignment.rounds_of_reviews)
      end

      self
    end

    # get answers and average scores for each team
    def summarize_reviews_by_team_reviewee(assignment, reviewee, rubric)
      self.summary[reviewee.name] = []
      self.avg_scores_by_reviewee[reviewee.name] = 0.0
      self.avg_scores_by_round[reviewee.name] = self.avg_scores_by_criterion[reviewee.name] = []

      # get the name of reviewers for display only
      self.reviewers[reviewee.name] = get_reviewers_by_reviewee_and_assignment(reviewee, assignment.id)

      # get answers and average scores of each round by rubric
      (0..assignment.rounds_of_reviews - 1).each do |round|
        self.summary[reviewee.name][round] = {}
        self.avg_scores_by_round[reviewee.name][round] = 0.0
        self.avg_scores_by_criterion[reviewee.name][round] = {}
        summarize_by_reviewee_round(assignment, reviewee, round, rubric)
      end
    end

    # get answers and averge score for each question in a round
    def summarize_by_reviewee_round(assignment, reviewee, round, rubric)
      threads = []
      # if use the same rubric, only use rubric[0]
      rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
      rubric_questions_used.each do |q|
        next if q.type.eql?("SectionHeader")

        # get all answers to this question
        question_answers = Answer.answers_by_question_for_reviewee_in_round(assignment.id, reviewee.id, q.id, round + 1)

        # get score and summary of answers for each question
        self.avg_scores_by_criterion[reviewee.name][round][q.txt] = calculate_avg_score_by_criterion(question_answers, get_max_score_for_question(q))

        threads << Thread.new do
          self.summary[reviewee.name][round][q.txt] = summarize_sentences(break_up_comments_to_sentences(question_answers), self.summary_ws_url)
        end
      end
      avg_scores_by_round = calculate_avg_score_by_round(self.avg_scores_by_criterion[reviewee.name][round], rubric_questions_used)
      self.avg_scores_by_round[reviewee.name][round] = avg_scores_by_round
      # Wait for all threads to end
      end_threads(threads)
    end

Impact:

Assignment Branch Condition size for summarize_reviews_by_reviewees is reduced from 89.93 to 16.64
Cognitive Complexity is reduced from 17 to 7

Refactor - summarize_sentence

This method calls web service to store each summary in a hashmap and use the question as the key.

Changes Made:

Removed variable summary

Refactor - break_up_comments_to_sentences

This method adds the comment to an array to be converted as a json request.

Changes Made:

The method is broken down into 2 smaller methods namely break_up_comments_to_sentences and get_sentences where get_sentences is called by break_up_comments_to_sentences to get sentences in desired format.

Impact:

The Cognitive complexity of break_up_comments_to_sentences reduced from 6 to <5

Refactor - get_questions_by_assignment

This method returns the rubric for given assignment

Changes Made:

In IF CONDITION: Removed unnecessary use of variable which was being used only once (questionaire_id) and replaced the variable with its assignment (assignment.review_questionnaire_id(round + 1)
In ELSE CONDITION: Removed unnecessary ternary operation for variable questionaire_id and replaced the variable with its assignment (assignment.review_questionnaire_id)

Refactor - calculate_avg_score_by_round

This method is used to calculate average round score for each question.

Changes Made:

Refactored the method into 2 smaller methods namely calculate_avg_score_by_round and calculate_round_score where calculate_avg_score_by_round calls calculate_round_score to calculate average round score and calculate_avg_score_by_round rounds the round_score upto 2 decimal places.

Before:

    def calculate_avg_score_by_round(avg_scores_by_criterion, criteria)
      round_score = 0.0
      sum_weight = 0

      criteria.each do |q|
        # include this score in the average round score if the weight is valid & q is criterion
        if !q.weight.nil? and q.weight > 0 and q.type.eql?("Criterion")
          round_score += avg_scores_by_criterion[q.txt] * q.weight
          sum_weight += q.weight
        end
      end

      round_score /= sum_weight if sum_weight > 0 and round_score > 0

      round_score.round(2)
    end

After:

    def calculate_round_score(avg_scores_by_criterion, criteria)
      round_score = sum_weight = 0.0
      criteria.each do |q|
        # include this score in the average round score if the weight is valid & q is criterion
        if !q.weight.nil? and q.weight > 0 and q.type.eql?("Criterion")
          round_score += avg_scores_by_criterion[q.txt] * q.weight
          sum_weight += q.weight
        end
      end

      round_score /= sum_weight if sum_weight > 0 and round_score > 0
      round_score
    end

    def calculate_avg_score_by_round(avg_scores_by_criterion, criteria)
      round_score = calculate_round_score(avg_scores_by_criterion, criteria)
      round_score.round(2)
    end

Impact:

Assignment Branch Condition size for calculate_avg_score_by_round is reduced from 18.57 to <15.

Coverage

Coverage increased (+17.1%) to 41.407%

Pull request

https://github.com/expertiza/expertiza/pull/1685

Test Plan

Manual Testing

Login Details: USERNAME: instructor6 PASSWORD: password

Click Assignment >> Click the View Submissions of Madeup problem >> Click on any student >> Click on Madeup problem >> Click on Your Scores

The 3 main functions of the Summary helper are summarize review by reviewees, summarize review by reviewee and summarize reviews by criterion.

To check summarize reviews by reviewees is working we should get the output similar to the one shown below. This function summarizes all the reviews and displays average score.

To check summarize reviews by reviewee is working, click on any review.

A new webpage pops up with all the reviews and scores given by an individual.

To check summarize reviews by criterion is working, click on any criterion. This should display summarized reviews and scores for a particular question in the questionnaire.

Future improvement

Modularize summarize_by_rounds to even smaller modules so that Assignment Branch Condition size is reduced from 36.73 to 15.00.
Create more test cases for the new modularized methods.

CSC/ECE 517 Spring 2020 - Project E2008. Refactor summary helper.rb

Contents

About Expertiza

Problem Statement

Issues found

Assignment Branch Condition

Cognitive Complexity

Method/line too long

Unused variables / access modifiers

Solution Implemented

Refactor - summarize_reviews_by_reviewee

Refactor - summarize_reviews_by_criterion

Refactor - summarize_reviews_by_reviewees

Refactor - summarize_sentence

Refactor - break_up_comments_to_sentences

Refactor - get_questions_by_assignment

Refactor - calculate_avg_score_by_round

Coverage

Pull request

Test Plan

Manual Testing

Future improvement

Contributors

Navigation menu

CSC/ECE 517 Spring 2020 - Project E2008. Refactor summary helper.rb

About Expertiza

Problem Statement

Issues found

Assignment Branch Condition

Cognitive Complexity

Method/line too long

Unused variables / access modifiers

Solution Implemented

Refactor - summarize_reviews_by_reviewee

Refactor - summarize_reviews_by_criterion

Refactor - summarize_reviews_by_reviewees

Refactor - summarize_sentence

Refactor - break_up_comments_to_sentences

Refactor - get_questions_by_assignment

Refactor - calculate_avg_score_by_round

Coverage

Pull request

Test Plan

Manual Testing

Future improvement

Contributors

Navigation menu

Search