CSC/ECE 517 Fall 2015/oss E1562 APS: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
(updated code snippet)
 
(80 intermediate revisions by 2 users not shown)
Line 1: Line 1:
'''E1562. Refactor AssignmentParticipant model'''<ref>https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus</ref>
'''E1562. Refactor AssignmentParticipant model'''<ref>Project Description document https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus</ref>


This page provides a description of the Expertiza based OSS project. This project aimed at refactoring the Leaderboard model, LeaderboardController, and Leaderboard_helper classes as per standard coding methodology for Ruby on Rails.
This page provides a brief description of the '''Expertiza''' project. The project is aimed at refactoring the AssignmentParticipant model which is subclass of Participant model. This model is used to maintain the list of students/users participating in a given assignment. For any new or existing assignments, this model manages the entire list of users assigned to that particular assignment. It primarily includes method for scores calculations, assignment submission paths, reviews, etc. As part of this project, some of the methods have been removed which were not being used anywhere, some have been moved to their appropriate helper module and some have been refactored into smaller ones.


'''Introduction to Expertiza'''<br>
==Introduction to Expertiza==
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.


[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system used to provide improved learning experience. Project is developed as a combined effort of students and faculty using the [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create and customize assignments, create a list of topics the students can sign up for, have students work on teams and then review each other's assignments at the end. Expertiza supports submission of almost any document type, including the URLs and wiki pages.
==Why refactoring?==
'''Refactoring'''<ref>Refactoring https://en.wikipedia.org/wiki/Code_refactoring</ref> is restructuring of code without the need of changing any external behavior. It reduces complexity and improves readability. It also becomes easy to extend the application with respect to different modules and their functionalities.
Some common techniques to refactor are:


== E162 Problem Statement ==
* Moving methods to appropriate modules
===Requirement===
* Breaking methods into more meaningful functionality
Refactor of '''AssignmentParticipant''' Model
* Creating more generalized code.
Classname: assignment_participant.rb
* Renaming methods and variable.
* Inheritance


===Tasks===
== Project Description ==
* Scores(questions) method is complex and huge. Refactor this method into smaller methods.
Refactor '''AssignmentParticipant''' model which is a subclass of '''Participant''' model.
* In set_handle method, there is no need for a separate elseif statement. Instead club the elseif statement into if statement as an another ‘or’ condition.
The following tasks have been performed as per the requirements.
* Some of the methods of this class are not used anywhere like compute_quiz_scores(scores) , average_score_per_assignment(assignment_id) . Remove such methods from the class.
* Method cycle_deviation_score(cycle) is also present in CollusionCycle, remove any such method from this class.
* Methods related to reviews like teammate_reviews, bookmark_reviews etc do not belong to AssignmentParticipant model. Move them to appropriate class.
* Methods related to files and directories like files(directory) should not be present in AssignmentParticipant model, move them to appropriate file helper modules.


== Introduction to Expertiza ==
* Refactor scores method to smaller methods.
* In set_handle method, there is no need for a separate ELSEIF statement. Club the ELSEIF statement into IF statement as an another OR condition.
* Remove methods like compute_quiz_scores(scores) , average_score_per_assignment(assignment_id) which are not being used.
* Method cycle_deviation_score(cycle) is also present in CollusionCycle, remove it from this class.
* Methods related to reviews like teammate_reviews, bookmark_reviews do not belong to AssignmentParticipant model. Move them to the appropriate class.
* Methods related to files and directories like files(directory) should not be present in AssignmentParticipant model, move them to FileHelper module.


== Refactoring ==
== Refactoring ==
===Scores method===
The method '''scores''' has been converted to smaller methods '''scores''', '''assignment_questionnaires''',  '''merge_scores''', '''calculate_scores'''. It is a good practice to keep the methods not extremely long as they tend to complicate the functionality and the readability.
{| class = "wikitable"
|-
!Before!!After
|-
|def scores(questions)
    scores = {}
    scores[:participant] = self
    self.assignment.questionnaires.each do |questionnaire|
      round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
      #create symbol for "varying rubrics" feature -Yang
      if(round!=nil)
        questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
      else
        questionnaire_symbol = questionnaire.symbol
      end
      scores[questionnaire_symbol] = {}
      if round==nil
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
      else
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
      end
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
    end
    scores[:total_score] = self.assignment.compute_total_score(scores)
    #merge scores[review#] (for each round) to score[review]  -Yang
    if self.assignment.varying_rubrics_by_round?
      review_sym = "review".to_sym
      scores[review_sym] = Hash.new
      scores[review_sym][:assessments] = Array.new
      scores[review_sym][:scores] = Hash.new
      scores[review_sym][:scores][:max] = -999999999
      scores[review_sym][:scores][:min] = 999999999
      scores[review_sym][:scores][:avg] = 0
      total_score = 0
      for i in 1..self.assignment.get_review_rounds
        round_sym = ("review"+i.to_s).to_sym
        if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
          next
        end
        length_of_assessments=scores[round_sym][:assessments].length.to_f
        scores[review_sym][:assessments]+=scores[round_sym][:assessments]
        if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
          scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
        end
        if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
          scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
        end
        if(scores[round_sym][:scores][:avg]!=nil)
          total_score += scores[round_sym][:scores][:avg]*length_of_assessments
        end
      end
      if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
              scores[review_sym][:scores][:max] = 0
              scores[review_sym][:scores][:min] = 0
      end
      scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
    end
    # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
    # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
    if assignment.is_microtask?
      topic = SignUpTopic.find_by_assignment_id(assignment.id)
      if !topic.nil?
        scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
        scores[:max_pts_available] = topic.micropayment
      end
    end
    # for all quiz questionnaires (quizzes) taken by the participant
    quiz_responses = Array.new
    quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
    quiz_response_mappings.each do |qmapping|
      if (qmapping.response)
        quiz_responses << qmapping.response
      end
    end
    #scores[:quiz] = Hash.new
    #scores[:quiz][:assessments] = quiz_responses
    #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
    scores[:total_score] = assignment.compute_total_score(scores)
    #scores[:total_score] += compute_quiz_scores(scores)
    # move lots of calculation from view(_participant.html.erb) to model
    if self.grade
      scores[:total_score] = self.grade
    else
      total_score = scores[:total_score]
      if total_score > 100
        total_score = 100
      end
      scores[:total_score] = total_score
    scores
    end
  end
||
* scores(questions) method
  def scores(questions)
    scores = {}
    scores[:participant] = self
    assignment_questionnaires(questions, scores)
    scores[:total_score] = self.assignment.compute_total_score(scores)
    merge_scores(scores)
    # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
    # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
    if assignment.is_microtask?
      topic = SignUpTopic.find_by_assignment_id(assignment.id)
      if !topic.nil?
        scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
        scores[:max_pts_available] = topic.micropayment
      end
    end
    # for all quiz questionnaires (quizzes) taken by the participant
    quiz_responses = Array.new
    quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
    quiz_response_mappings.each do |qmapping|
      if (qmapping.response)
        quiz_responses << qmapping.response
      end
    end
    #scores[:quiz] = Hash.new
    #scores[:quiz][:assessments] = quiz_responses
    #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
    scores[:total_score] = assignment.compute_total_score(scores)
    #scores[:total_score] += compute_quiz_scores(scores)
    calculate_scores(scores)
  end
* assignment_questionnaires(questions, scores) method
  def assignment_questionnaires(questions, scores)
    self.assignment.questionnaires.each do |questionnaire|
      round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
      #create symbol for "varying rubrics" feature -Yang
      if(round!=nil)
        questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
      else
        questionnaire_symbol = questionnaire.symbol
      end
      scores[questionnaire_symbol] = {}
      if round==nil
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
      else
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
      end
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
    end
  end
* merge_scores(scores) method
  def merge_scores(scores)
    #merge scores[review#] (for each round) to score[review]  -Yang
    if self.assignment.varying_rubrics_by_round?
      review_sym = "review".to_sym
      scores[review_sym] = Hash.new
      scores[review_sym][:assessments] = Array.new
      scores[review_sym][:scores] = Hash.new
      scores[review_sym][:scores][:max] = -999999999
      scores[review_sym][:scores][:min] = 999999999
      scores[review_sym][:scores][:avg] = 0
      total_score = 0
      for i in 1..self.assignment.get_review_rounds
        round_sym = ("review"+i.to_s).to_sym
        if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
          next
        end
        length_of_assessments=scores[round_sym][:assessments].length.to_f
        scores[review_sym][:assessments]+=scores[round_sym][:assessments]
        if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
          scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
        end
        if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
          scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
        end
        if(scores[round_sym][:scores][:avg]!=nil)
          total_score += scores[round_sym][:scores][:avg]*length_of_assessments
        end
      end
      if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
        scores[review_sym][:scores][:max] = 0
        scores[review_sym][:scores][:min] = 0
      end
      scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
    end
  end
* calculate_scores(scores) method
  def calculate_scores(scores)
    # move lots of calculation from view(_participant.html.erb) to model
    if self.grade
      scores[:total_score] = self.grade
    else
      total_score = scores[:total_score]
      if total_score > 100
        total_score = 100
      end
      scores[:total_score] = total_score
      scores
    end
  end
|-
|}
===set_handle method===
The set_handle method had an unncessary ELSIF statement which was performing the same action '''self.handle = self.user.name'''. This has been clubbed as an OR condition to the IF statement and the first ELSIF statement has been removed.
{| class = "wikitable"
|-
!Before!!After
|-
|def set_handle
      if self.user.handle == nil or self.user.handle == ""
        self.handle = self.user.name
      elsif AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
        self.handle = self.user.name
      else
        self.handle = self.user.handle
      end
      self.save!
  end
||def set_handle
      if self.user.handle == nil or self.user.handle == "" or AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
        self.handle = self.user.name
      else
        self.handle = self.user.handle
      end
      self.save!
  end
|-
|}
===File and directory methods===
The files and directory methods from AssignmentParticipant model have been moved to the '''FileHelper''' module.
The methods '''files''', '''submitted_files''' and '''dir_path''' are appropriate for the '''FileHelper' module and hence been moved there.
The below line was added to the AssignmentParticipant model.
include FileHelper
The AssignmentParticipant class would look like
  class AssignmentParticipant < Participant
    require 'wiki_helper'
    '''include FileHelper'''
Methods moved to FileHelper:
* submitted_files
  def submitted_files
    files(self.path) if self.directory_num
  end
* files(directory)
  def files(directory)
    files_list = Dir[directory + "/*"]
    files = Array.new
    files_list.each do |file|
      if File.directory?(file)
        dir_files = files(file)
        dir_files.each{|f| files << f}
      end
      files << file
    end
    files
  end
* submitted_files()
  def submitted_files()
    files = Array.new
    if(self.directory_num)
      files = files(self.path)
    end
    return files
  end
* dir_path
  def dir_path
    assignment.try :directory_path
  end
===Remove unused methods===
The following methods have been removed as they were not being used by the application.
'''def average_score_per_assignment(assignment_id)'''
def average_score_per_assignment(assignment_id)
    return 0 if self.response_maps.size == 0
    sum_of_scores = 0
    self.response_maps.metareview_response_maps.each do |metaresponse_map|
      if !metaresponse_map.response.empty? && response_map == assignment_id then
        sum_of_scores = sum_of_scores + response_map.response.last.average_score
      end
    end
    (sum_of_scores / self.response_maps.size).to_i
  end
'''def quiz_taken_by?(contributor, reviewer)'''
  def quiz_taken_by?(contributor, reviewer)
    quiz_id = QuizQuestionnaire.find_by_instructor_id(contributor.id)
    return QuizResponseMap.where(['reviewee_id = ? AND reviewer_id = ? AND reviewed_object_id = ?',
                                  self.id, reviewer.id, quiz_id]).count > 0
  end
'''def has_quiz?'''
  def has_quiz?
    return !QuizQuestionnaire.find_by_instructor_id(self.id).nil?
  end
'''def reviewees'''
  def reviewees
    reviewees = []
    rmaps = ResponseMap.all(conditions: ["reviewer_id = #{self.id} && type = 'ReviewResponseMap'"])
        rmaps.each { |rm| reviewees.concat(AssignmentTeam.find(rm.reviewee_id).participants) }
    reviewees
  end
'''def two_node_cycles'''
  def two_node_cycles
    cycles = []
    self.reviewers.each do |ap|
      if ap.reviewers.include?(self)
        self.reviews_by_reviewer(ap).nil? ? next : s01 = self.reviews_by_reviewer(ap).get_total_score
        ap.reviews_by_reviewer(self).nil? ? next : s10 = ap.reviews_by_reviewer(self).get_total_score
        cycles.push([[self, s01], [ap, s10]])
      end
    end
    cycles
  end
'''def three_node_cycles'''
  def three_node_cycles
    cycles = []
    self.reviewers.each do |ap1|
      ap1.reviewers.each do |ap2|
        if ap2.reviewers.include?(self)
          self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
          ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
          ap2.reviews_by_reviewer(self).nil? ? next : s20 = ap2.reviews_by_reviewer(self).get_total_score
          cycles.push([[self, s01], [ap1, s12], [ap2, s20]])
        end
      end
    end
    cycles
  end
'''def four_node_cycles'''
  def four_node_cycles
    cycles = []
    self.reviewers.each do |ap1|
      ap1.reviewers.each do |ap2|
        ap2.reviewers.each do |ap3|
          if ap3.reviewers.include?(self)
            self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
            ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
            ap2.reviews_by_reviewer(ap3).nil? ? next : s23 = ap2.reviews_by_reviewer(ap3).get_total_score
            ap3.reviews_by_reviewer(self).nil? ? next : s30 = ap3.reviews_by_reviewer(self).get_total_score
            cycles.push([[self, s01], [ap1, s12], [ap2, s23], [ap3, s30]])
          end
        end
      end
    end
    cycles
  end
'''def cycle_similarity_score(cycle)'''
  def cycle_similarity_score(cycle)
    similarity_score = 0.0
    count = 0.0
    0 ... cycle.size-1.each do |pivot|
      pivot_score = cycle[pivot][1]
      similarity_score = similarity_score + (pivot_score - cycle[other][1]).abs
      count = count + 1.0
    end
    similarity_score = similarity_score / count unless count == 0.0
    similarity_score
  end
'''def cycle_deviation_score(cycle)'''
  def cycle_deviation_score(cycle)
    deviation_score = 0.0
    count = 0.0
    0 ... cycle.size.each do |member|
      participant = AssignmentParticipant.find(cycle[member][0].id)
      total_score = participant.get_review_score
      deviation_score = deviation_score + (total_score - cycle[member][1]).abs
      count = count + 1.0
    end
    deviation_score = deviation_score / count unless count == 0.0
    deviation_score
  end
'''def compute_quiz_scores(scores)'''
  def compute_quiz_scores(scores)
    total = 0
    if scores[:quiz][:scores][:avg]
      return scores[:quiz][:scores][:avg] * 100  / 100.to_f
    else
      return 0
    end
    return total
  end
'''def review_response_maps'''
  def review_response_maps
      participant = Participant.find(id)
      team_id = TeamsUser.team_id(participant.parent_id, participant.user_id)
      ReviewResponseMap.where(reviewee_id: team_id, reviewed_object_id: assignment.id)
  end
'''def members'''
  def members
    team.try :participants
  end
'''def get_hash(time_stamp)'''
  def get_hash(time_stamp)
    # first generate a hash from the assignment name itself
    hash_data = Digest::SHA1.digest(self.assignment.name.to_s)
    # second generate a hash from the first hash plus the user name and time stamp
    sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S")
    Digest::SHA1.digest(sign)
  end
===Review methods===
* The bookmark_reviews() method from AssignmentParticipant was being called once in '''get_assessments_for''' method of '''BookmarkRatingQuestionnaire''' class. It has been replaced by the method statement directly since it only had a single line of code.
{| class = "wikitable"
|-
!Before!!After
|-
|def bookmark_reviews
    BookmarkRatingResponseMap.get_assessments_for(self)
  end
||def get_assessments_for(participant)
    BookmarkRatingResponseMap.get_assessments_for(participant) # participant.bookmark_reviews()
  end
|-
|}
* The teammate_reviews() method from AssignmentParticipant was being called once in '''get_assessments_for''' method of '''TeammateReviewQuestionnaire''' class. It has been replaced by the method statement directly since it only had a single line of code.
{| class = "wikitable"
|-
!Before!!After
|-
|def teammate_reviews
    TeammateReviewResponseMap.get_assessments_for(self)
  end
||def get_assessments_for(participant)
    TeammateReviewResponseMap.get_assessments_for(participant)  #participant.teammate_reviews()
  end
|-
|}


== Testing ==
== Testing ==
Please follow the below instruction to test UI:
* Login to Expertiza on http://152.46.20.199:3000/.
* Enter username as 'student13' and password as 'password'.
* You can select a previous assignment 'shivam test' or 'Pankti test'.
* You should be able to access the pages further
** Your team- The page content should be visible without any errors.
** Your work - The upload button for submission should be visible.
** Your scores- The page should give result 0 and not any error.
When the user runs the application, the behavior of the application remains the same.
The existing [https://en.wikipedia.org/wiki/RSpec RSpec] test cases remain successful. The same can be checked by running
Rspec spec
The participant is able to log in to the system, view the assignments, submit links and files in assignment submission, view the scores, review peers, etc. Below are some of the execution screenshots after the refactoring of the model.
The Expertiza Login Screen
[[File:ExpertizaLoginScreen.jpg]]
User can see all the assignments as below.
[[File:ExpertizaAPS2.jpg]]
View team members for the assignment selected. In this case, we have taken a dummy assignment we created.
[[File:ExpertizaAPS3.jpg]]
View the submission page to upload the assignment.
[[File:ExpertizaAPS4.jpg]]
View the scores of an assignment.
[[File:ExpertizaAPS5.jpg]]
==Resources==
* Expertiza Github repository<ref>Expertiza Github repository https://github.com/expertiza/expertiza</ref>
* Our Github repository<ref>Our Github Repository https://github.com/shivamgulati1991/expertiza]</ref>


== References ==
== References ==
<references/>
<references/>

Latest revision as of 16:06, 6 November 2015

E1562. Refactor AssignmentParticipant model<ref>Project Description document https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus</ref>

This page provides a brief description of the Expertiza project. The project is aimed at refactoring the AssignmentParticipant model which is subclass of Participant model. This model is used to maintain the list of students/users participating in a given assignment. For any new or existing assignments, this model manages the entire list of users assigned to that particular assignment. It primarily includes method for scores calculations, assignment submission paths, reviews, etc. As part of this project, some of the methods have been removed which were not being used anywhere, some have been moved to their appropriate helper module and some have been refactored into smaller ones.

Introduction to Expertiza

Expertiza is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using Ruby on Rails framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments. Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.

Why refactoring?

Refactoring<ref>Refactoring https://en.wikipedia.org/wiki/Code_refactoring</ref> is restructuring of code without the need of changing any external behavior. It reduces complexity and improves readability. It also becomes easy to extend the application with respect to different modules and their functionalities. Some common techniques to refactor are:

  • Moving methods to appropriate modules
  • Breaking methods into more meaningful functionality
  • Creating more generalized code.
  • Renaming methods and variable.
  • Inheritance

Project Description

Refactor AssignmentParticipant model which is a subclass of Participant model. The following tasks have been performed as per the requirements.

  • Refactor scores method to smaller methods.
  • In set_handle method, there is no need for a separate ELSEIF statement. Club the ELSEIF statement into IF statement as an another OR condition.
  • Remove methods like compute_quiz_scores(scores) , average_score_per_assignment(assignment_id) which are not being used.
  • Method cycle_deviation_score(cycle) is also present in CollusionCycle, remove it from this class.
  • Methods related to reviews like teammate_reviews, bookmark_reviews do not belong to AssignmentParticipant model. Move them to the appropriate class.
  • Methods related to files and directories like files(directory) should not be present in AssignmentParticipant model, move them to FileHelper module.

Refactoring

Scores method

The method scores has been converted to smaller methods scores, assignment_questionnaires, merge_scores, calculate_scores. It is a good practice to keep the methods not extremely long as they tend to complicate the functionality and the readability.

Before After
def scores(questions)
   scores = {}
   scores[:participant] = self
   self.assignment.questionnaires.each do |questionnaire|
     round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
     #create symbol for "varying rubrics" feature -Yang
     if(round!=nil)
       questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
     else
       questionnaire_symbol = questionnaire.symbol
     end
     scores[questionnaire_symbol] = {}
     if round==nil
       scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
     else
       scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
     end
     scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
   end
   scores[:total_score] = self.assignment.compute_total_score(scores)
   #merge scores[review#] (for each round) to score[review]  -Yang
   if self.assignment.varying_rubrics_by_round?
     review_sym = "review".to_sym
     scores[review_sym] = Hash.new
     scores[review_sym][:assessments] = Array.new
     scores[review_sym][:scores] = Hash.new
     scores[review_sym][:scores][:max] = -999999999
     scores[review_sym][:scores][:min] = 999999999
     scores[review_sym][:scores][:avg] = 0
     total_score = 0
     for i in 1..self.assignment.get_review_rounds
       round_sym = ("review"+i.to_s).to_sym
       if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
         next
       end
       length_of_assessments=scores[round_sym][:assessments].length.to_f
       scores[review_sym][:assessments]+=scores[round_sym][:assessments]
       if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
         scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
       end
       if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
         scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
       end
       if(scores[round_sym][:scores][:avg]!=nil)
         total_score += scores[round_sym][:scores][:avg]*length_of_assessments
       end
     end
     if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
              scores[review_sym][:scores][:max] = 0
              scores[review_sym][:scores][:min] = 0
     end
     scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
   end
   # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
   # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
   if assignment.is_microtask?
     topic = SignUpTopic.find_by_assignment_id(assignment.id)
     if !topic.nil?
       scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
       scores[:max_pts_available] = topic.micropayment
     end
   end
   # for all quiz questionnaires (quizzes) taken by the participant
   quiz_responses = Array.new
   quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
   quiz_response_mappings.each do |qmapping|
     if (qmapping.response)
       quiz_responses << qmapping.response
     end
   end
   #scores[:quiz] = Hash.new
   #scores[:quiz][:assessments] = quiz_responses
   #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
   scores[:total_score] = assignment.compute_total_score(scores)
   #scores[:total_score] += compute_quiz_scores(scores)
   # move lots of calculation from view(_participant.html.erb) to model
   if self.grade
     scores[:total_score] = self.grade
   else
     total_score = scores[:total_score]
     if total_score > 100
       total_score = 100
     end
     scores[:total_score] = total_score
   scores
   end
 end
  • scores(questions) method
 def scores(questions)
   scores = {}
   scores[:participant] = self
   assignment_questionnaires(questions, scores)
   scores[:total_score] = self.assignment.compute_total_score(scores)
   merge_scores(scores)
   # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
   # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
   if assignment.is_microtask?
     topic = SignUpTopic.find_by_assignment_id(assignment.id)
     if !topic.nil?
       scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
       scores[:max_pts_available] = topic.micropayment
     end
   end
   # for all quiz questionnaires (quizzes) taken by the participant
   quiz_responses = Array.new
   quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
   quiz_response_mappings.each do |qmapping|
     if (qmapping.response)
       quiz_responses << qmapping.response
     end
   end
   #scores[:quiz] = Hash.new
   #scores[:quiz][:assessments] = quiz_responses
   #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
   scores[:total_score] = assignment.compute_total_score(scores)
   #scores[:total_score] += compute_quiz_scores(scores)
   calculate_scores(scores)
 end
  • assignment_questionnaires(questions, scores) method
 def assignment_questionnaires(questions, scores)
   self.assignment.questionnaires.each do |questionnaire|
     round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
     #create symbol for "varying rubrics" feature -Yang
     if(round!=nil)
       questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
     else
       questionnaire_symbol = questionnaire.symbol
     end
     scores[questionnaire_symbol] = {}
     if round==nil
       scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
     else
       scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
     end
     scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
   end
 end
  • merge_scores(scores) method
 def merge_scores(scores)
   #merge scores[review#] (for each round) to score[review]  -Yang
   if self.assignment.varying_rubrics_by_round?
     review_sym = "review".to_sym
     scores[review_sym] = Hash.new
     scores[review_sym][:assessments] = Array.new
     scores[review_sym][:scores] = Hash.new
     scores[review_sym][:scores][:max] = -999999999
     scores[review_sym][:scores][:min] = 999999999
     scores[review_sym][:scores][:avg] = 0
     total_score = 0
     for i in 1..self.assignment.get_review_rounds
       round_sym = ("review"+i.to_s).to_sym
       if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
         next
       end
       length_of_assessments=scores[round_sym][:assessments].length.to_f
       scores[review_sym][:assessments]+=scores[round_sym][:assessments]
       if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
         scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
       end
       if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
         scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
       end
       if(scores[round_sym][:scores][:avg]!=nil)
         total_score += scores[round_sym][:scores][:avg]*length_of_assessments
       end
     end
     if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
       scores[review_sym][:scores][:max] = 0
       scores[review_sym][:scores][:min] = 0
     end
     scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
   end
 end
  • calculate_scores(scores) method
 def calculate_scores(scores)
   # move lots of calculation from view(_participant.html.erb) to model
   if self.grade
     scores[:total_score] = self.grade
   else
     total_score = scores[:total_score]
     if total_score > 100
       total_score = 100
     end
     scores[:total_score] = total_score
     scores
   end
 end

set_handle method

The set_handle method had an unncessary ELSIF statement which was performing the same action self.handle = self.user.name. This has been clubbed as an OR condition to the IF statement and the first ELSIF statement has been removed.

Before After
def set_handle
     if self.user.handle == nil or self.user.handle == ""
       self.handle = self.user.name
     elsif AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
       self.handle = self.user.name
     else
       self.handle = self.user.handle
     end
     self.save!
 end
def set_handle
     if self.user.handle == nil or self.user.handle == "" or AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
       self.handle = self.user.name
     else
       self.handle = self.user.handle
     end
     self.save!
 end

File and directory methods

The files and directory methods from AssignmentParticipant model have been moved to the FileHelper module. The methods files, submitted_files and dir_path are appropriate for the FileHelper' module and hence been moved there. The below line was added to the AssignmentParticipant model. include FileHelper

The AssignmentParticipant class would look like

 class AssignmentParticipant < Participant
   require 'wiki_helper'
   include FileHelper

Methods moved to FileHelper:

  • submitted_files
 def submitted_files
   files(self.path) if self.directory_num
 end
  • files(directory)
 def files(directory)
   files_list = Dir[directory + "/*"]
   files = Array.new
   files_list.each do |file|
     if File.directory?(file)
       dir_files = files(file)
       dir_files.each{|f| files << f}
     end
     files << file
   end
   files
 end
  • submitted_files()
 def submitted_files()
   files = Array.new
   if(self.directory_num)
     files = files(self.path)
   end
   return files
 end
  • dir_path
 def dir_path
   assignment.try :directory_path
 end

Remove unused methods

The following methods have been removed as they were not being used by the application.

def average_score_per_assignment(assignment_id)

def average_score_per_assignment(assignment_id)
   return 0 if self.response_maps.size == 0
   sum_of_scores = 0
   self.response_maps.metareview_response_maps.each do |metaresponse_map|
     if !metaresponse_map.response.empty? && response_map == assignment_id then
       sum_of_scores = sum_of_scores + response_map.response.last.average_score
     end
   end
   (sum_of_scores / self.response_maps.size).to_i
 end


def quiz_taken_by?(contributor, reviewer)

 def quiz_taken_by?(contributor, reviewer)
   quiz_id = QuizQuestionnaire.find_by_instructor_id(contributor.id)
   return QuizResponseMap.where(['reviewee_id = ? AND reviewer_id = ? AND reviewed_object_id = ?',
                                 self.id, reviewer.id, quiz_id]).count > 0
 end


def has_quiz?

 def has_quiz?
   return !QuizQuestionnaire.find_by_instructor_id(self.id).nil?
 end


def reviewees

 def reviewees
   reviewees = []
   rmaps = ResponseMap.all(conditions: ["reviewer_id = #{self.id} && type = 'ReviewResponseMap'"])
       rmaps.each { |rm| reviewees.concat(AssignmentTeam.find(rm.reviewee_id).participants) }
   reviewees
 end


def two_node_cycles

 def two_node_cycles
   cycles = []
   self.reviewers.each do |ap|
     if ap.reviewers.include?(self)
       self.reviews_by_reviewer(ap).nil? ? next : s01 = self.reviews_by_reviewer(ap).get_total_score
       ap.reviews_by_reviewer(self).nil? ? next : s10 = ap.reviews_by_reviewer(self).get_total_score
       cycles.push([[self, s01], [ap, s10]])
     end
   end
   cycles
 end


def three_node_cycles

 def three_node_cycles
   cycles = []
   self.reviewers.each do |ap1|
     ap1.reviewers.each do |ap2|
       if ap2.reviewers.include?(self)
         self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
         ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
         ap2.reviews_by_reviewer(self).nil? ? next : s20 = ap2.reviews_by_reviewer(self).get_total_score
         cycles.push([[self, s01], [ap1, s12], [ap2, s20]])
       end
     end
   end
   cycles
 end


def four_node_cycles

 def four_node_cycles
   cycles = []
   self.reviewers.each do |ap1|
     ap1.reviewers.each do |ap2|
       ap2.reviewers.each do |ap3|
         if ap3.reviewers.include?(self)
           self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
           ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
           ap2.reviews_by_reviewer(ap3).nil? ? next : s23 = ap2.reviews_by_reviewer(ap3).get_total_score
           ap3.reviews_by_reviewer(self).nil? ? next : s30 = ap3.reviews_by_reviewer(self).get_total_score
           cycles.push([[self, s01], [ap1, s12], [ap2, s23], [ap3, s30]])
         end
       end
     end
   end
   cycles
 end


def cycle_similarity_score(cycle)

 def cycle_similarity_score(cycle)
   similarity_score = 0.0
   count = 0.0
   0 ... cycle.size-1.each do |pivot|
     pivot_score = cycle[pivot][1]
     similarity_score = similarity_score + (pivot_score - cycle[other][1]).abs
     count = count + 1.0
   end
   similarity_score = similarity_score / count unless count == 0.0
   similarity_score
 end


def cycle_deviation_score(cycle)

 def cycle_deviation_score(cycle)
   deviation_score = 0.0
   count = 0.0
   0 ... cycle.size.each do |member|
     participant = AssignmentParticipant.find(cycle[member][0].id)
     total_score = participant.get_review_score
     deviation_score = deviation_score + (total_score - cycle[member][1]).abs
     count = count + 1.0
   end
   deviation_score = deviation_score / count unless count == 0.0
   deviation_score
 end


def compute_quiz_scores(scores)

 def compute_quiz_scores(scores)
   total = 0
   if scores[:quiz][:scores][:avg]
     return scores[:quiz][:scores][:avg] * 100  / 100.to_f
   else
     return 0
   end
   return total
 end


def review_response_maps

 def review_response_maps
     participant = Participant.find(id)
     team_id = TeamsUser.team_id(participant.parent_id, participant.user_id)
     ReviewResponseMap.where(reviewee_id: team_id, reviewed_object_id: assignment.id)
 end


def members

 def members
   team.try :participants
 end


def get_hash(time_stamp)

 def get_hash(time_stamp)
   # first generate a hash from the assignment name itself
   hash_data = Digest::SHA1.digest(self.assignment.name.to_s)
   # second generate a hash from the first hash plus the user name and time stamp
   sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S")
   Digest::SHA1.digest(sign)
 end

Review methods

  • The bookmark_reviews() method from AssignmentParticipant was being called once in get_assessments_for method of BookmarkRatingQuestionnaire class. It has been replaced by the method statement directly since it only had a single line of code.
Before After
def bookmark_reviews
   BookmarkRatingResponseMap.get_assessments_for(self)
 end
def get_assessments_for(participant)
   BookmarkRatingResponseMap.get_assessments_for(participant) # participant.bookmark_reviews()
 end
  • The teammate_reviews() method from AssignmentParticipant was being called once in get_assessments_for method of TeammateReviewQuestionnaire class. It has been replaced by the method statement directly since it only had a single line of code.
Before After
def teammate_reviews
   TeammateReviewResponseMap.get_assessments_for(self)
 end
def get_assessments_for(participant)
   TeammateReviewResponseMap.get_assessments_for(participant)   #participant.teammate_reviews()
 end

Testing

Please follow the below instruction to test UI:

  • Login to Expertiza on http://152.46.20.199:3000/.
  • Enter username as 'student13' and password as 'password'.
  • You can select a previous assignment 'shivam test' or 'Pankti test'.
  • You should be able to access the pages further
    • Your team- The page content should be visible without any errors.
    • Your work - The upload button for submission should be visible.
    • Your scores- The page should give result 0 and not any error.

When the user runs the application, the behavior of the application remains the same.

The existing RSpec test cases remain successful. The same can be checked by running Rspec spec The participant is able to log in to the system, view the assignments, submit links and files in assignment submission, view the scores, review peers, etc. Below are some of the execution screenshots after the refactoring of the model.

The Expertiza Login Screen

User can see all the assignments as below.

View team members for the assignment selected. In this case, we have taken a dummy assignment we created.

View the submission page to upload the assignment.

View the scores of an assignment.

Resources

References

<references/>