CSC/ECE 517 Fall 2017/E1770 Refactor assignment participant.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
 
(47 intermediate revisions by the same user not shown)
Line 1: Line 1:
'''E1770. [Test First Development] Refactor assignment_participant.rb'''<ref>Project Description document https://docs.google.com/document/d/1rdolBAHxVGI9I0N-cT866AqnfORM2L1_m_bo2gRYRrI/edit#heading=h.s18ldps2kz5e</ref>
'''E1770. [Test First Development] Refactor assignment_participant.rb'''<ref>Project Description document https://docs.google.com/document/d/1rdolBAHxVGI9I0N-cT866AqnfORM2L1_m_bo2gRYRrI/edit#heading=h.s18ldps2kz5e</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.
This page provides a brief description of the '''Expertiza''' project. The project is aimed at using TFD(Test First Development) to refactor 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.
 
We used the TDF(Test First Development) method to refactor the AssignmentParticipant model. With the  RSpec tool, we wrote 38 test cases for 24 methods in '''assignment_participant_spec.rb''' first. After finishing these test cases, we executed them, and most of them failed. And then we refactored the methods, '''scores''', '''files''', '''self.import''', '''find_by''', '''where.first''', and executed the test cases again, finally they all passed.


==Introduction to Expertiza==
==Introduction to Expertiza==
Line 12: Line 14:
Refactoring improves nonfunctional attributes of the software. Advantages include improved code readability and reduced complexity; these can improve source-code maintainability and create a more expressive internal architecture or object model to improve extensibility. Typically, refactoring applies a series of standardised basic micro-refactorings, each of which is (usually) a tiny change in a computer program's source code that either preserves the behaviour of the software, or at least does not modify its conformance to functional requirements. Many development environments provide automated support for performing the mechanical aspects of these basic refactorings. If done extremely well, code refactoring may also resolve hidden, dormant, or undiscovered bugs or vulnerabilities in the system by simplifying the underlying logic and eliminating unnecessary levels of complexity. If done poorly it may fail the requirement that external functionality not be changed, introduce new bugs, or both.
Refactoring improves nonfunctional attributes of the software. Advantages include improved code readability and reduced complexity; these can improve source-code maintainability and create a more expressive internal architecture or object model to improve extensibility. Typically, refactoring applies a series of standardised basic micro-refactorings, each of which is (usually) a tiny change in a computer program's source code that either preserves the behaviour of the software, or at least does not modify its conformance to functional requirements. Many development environments provide automated support for performing the mechanical aspects of these basic refactorings. If done extremely well, code refactoring may also resolve hidden, dormant, or undiscovered bugs or vulnerabilities in the system by simplifying the underlying logic and eliminating unnecessary levels of complexity. If done poorly it may fail the requirement that external functionality not be changed, introduce new bugs, or both.


==Test-driven development==
==Why Test-driven development?==
'''Test-driven development (TDD)'''<ref>Refactoring https://en.wikipedia.org/wiki/Test-driven_development</ref> is a software development process that relies on the repetition of a very short development cycle: Requirements are turned into very specific test cases, then the software is improved to pass the new tests, only. This is opposed to software development that allows software to be added that is not proven to meet requirements.
'''Test-driven development (TDD)'''<ref>Test-driven development (TDD) https://en.wikipedia.org/wiki/Test-driven_development</ref> is a software development process that relies on the repetition of a very short development cycle: Requirements are turned into very specific test cases, then the software is improved to pass the new tests, only. This is opposed to software development that allows software to be added that is not proven to meet requirements. The followings are several benefits of Test-driven development (TDD).
 
* Maintainable, Flexible, Easily Extensible.
* Unparalleled Test Coverage & Streamlined Codebase.
* Clean Interface.
* Refactoring Encourages Improvements.
* Executable Documentation.
 
==Introduction to  RSpec==
'''RSpec'''<ref>RSpec https://en.wikipedia.org/wiki/RSpec</ref> is a 'Domain Specific Language' (DSL) testing tool written in Ruby to test Ruby code. It is a behavior-driven development (BDD) framework which is extensively used in the production applications. The basic idea behind this concept is that of Test Driven Development(TDD) where the tests are written first and the development is based on writing just enough code that will fulfill those tests followed by refactoring. It contains its own mocking framework that is fully integrated into the framework based upon JMock. The simplicity in the RSpec syntax makes it one of the popular testing tools for Ruby applications. The RSpec tool can be used by installing the rspec gem which consists of 3 other gems namely rspec-core , rspec-expectation and rspec-mock.


== Project Description ==
== Project Description ==
Line 39: Line 50:
** Write failing tests first.
** Write failing tests first.


== Refactoring ==
== Test Cases ==
===Scores method===
Because the project is aimed at using TFD(Test First Development) to refactor the AssignmentParticipant model.
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.
We wrote 38 test cases for 24 methods in '''assignment_participant_spec.rb''' first, with the RSpec tool.


{| class = "wikitable"
The introduction video to our project<ref>Introduction video to our project https://www.youtube.com/watch?v=sJdmN-S2Voo&feature=youtu.be</ref>
|-
  describe '#dir_path' do
!Before!!After
     it 'returns the directory path of current assignment' do
|-
       expect(participant.dir_path).to eq "final_test"
|def scores(questions)
    end
    scores = {}
  end
    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] = {}
  describe '#assign_quiz' do
    it 'creates a new QuizResponseMap record' do
       allow(QuizQuestionnaire).to receive(:find_by).with(instructor_id: 1).and_return(quiz_questionaire) # WHY CAN NOT DELETE THIS SENTENCE
      expect { participant.assign_quiz(participant, participant2, nil) }.to change { QuizResponseMap.count }.from(0).to(1)
      expect(participant.assign_quiz(participant, participant2, nil)).to be_an_instance_of(QuizResponseMap)
    end
  end


       if round==nil
  describe '#reviewers' do
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
    it 'returns all the participants in this assignment who have reviewed the team where this participant belongs' do
       else
       allow(ReviewResponseMap).to receive(:where).with(any_args).and_return([response_map]) # differences with .with(parameter)
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
       allow(AssignmentParticipant).to receive(:find).with(any_args).and_return(participant2)
       end
       expect(participant.reviewers).to eq([participant2])
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
     end
     end
  end


    scores[:total_score] = self.assignment.compute_total_score(scores)
  describe '#review_score' do
 
     it 'returns the review score' do
     #merge scores[review#] (for each round) to score[review]  -Yang
      # diao yong de method tai duo le,er qie bu zhi dao sha yi si
    if self.assignment.varying_rubrics_by_round?
      allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
      review_sym = "review".to_sym
       allow(review_questionnaire).to receive(:questions).and_return(question)
      scores[review_sym] = Hash.new
       allow(Answer).to receive(:compute_scores).with([response], question).and_return(avg: 100)
       scores[review_sym][:assessments] = Array.new
       allow(review_questionnaire).to receive(:max_possible_score).and_return(100)
       scores[review_sym][:scores] = Hash.new
       expect(participant.review_score).to eq(100)
      scores[review_sym][:scores][:max] = -999999999
    end
       scores[review_sym][:scores][:min] = 999999999
  end
       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]
  describe '#scores' do
 
    context 'when assignment is not varying rubric by round and not an microtask' do
         if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
      it 'calculates scores that this participant has been given' do
          scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
         expect(participant).to receive(:assignment_questionnaires)
         end
        expect(assignment).to receive(:compute_total_score)
         if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
         allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
          scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
         allow(assignment).to receive(:is_microtask?).and_return(false)
         end
        expect(assignment).to receive(:compute_total_score)
        if(scores[round_sym][:scores][:avg]!=nil)
         expect(participant).to receive(:caculate_scores)
          total_score += scores[round_sym][:scores][:avg]*length_of_assessments
        participant.scores(question)
        end
       end
       end
    end


       if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
    context 'when assignment is varying rubric by round but not an microtask' do
              scores[review_sym][:scores][:max] = 0
       it 'calculates scores that this participant has been given' do
              scores[review_sym][:scores][:min] = 0
        expect(participant).to receive(:assignment_questionnaires)
        expect(assignment).to receive(:compute_total_score)
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(true)
        expect(participant).to receive(:merge_scores)
        allow(assignment).to receive(:is_microtask?).and_return(false)
        expect(assignment).to receive(:compute_total_score)
        expect(participant).to receive(:caculate_scores)
        participant.scores(question)
       end
       end
      scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
     end
     end


     # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
     context 'when assignment is not varying rubric by round but an microtask' do
    # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
      it 'calculates scores that this participant has been given' do
    if assignment.is_microtask?
        expect(participant).to receive(:assignment_questionnaires)
      topic = SignUpTopic.find_by_assignment_id(assignment.id)
        expect(assignment).to receive(:compute_total_score)
      if !topic.nil?
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
         scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
        allow(assignment).to receive(:is_microtask?).and_return(true)
         scores[:max_pts_available] = topic.micropayment
         expect(participant).to receive(:topic_total_scores)
        expect(assignment).to receive(:compute_total_score)
        expect(participant).to receive(:caculate_scores)
         participant.scores(question)
       end
       end
     end
     end
  end


     # for all quiz questionnaires (quizzes) taken by the participant
  # included method
    quiz_responses = Array.new
  describe '#assignment_questionnaires' do
    quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
     context 'when the round of questionnaire is nil' do
    quiz_response_mappings.each do |qmapping|
      it 'record the result as review scores' do
      if (qmapping.response)
        scores = {}
         quiz_responses << qmapping.response
        question_hash = {review: question}
        score_map = {max: 100, min: 100, avg: 100}
        allow(AssignmentQuestionnaire).to receive(:find_by).with(any_args).and_return(assignment_questionnaire)
        allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
        allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
        participant.assignment_questionnaires(question_hash, scores)
        expect(scores[:review][:assessments]).to eq([response])
         expect(scores[:review][:scores]).to eq(score_map)
       end
       end
     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
     context 'when the round of questionnaire is not nil' do
    if self.grade
      it 'record the result as review#{n} scores' do
      scores[:total_score] = self.grade
        scores = {}
    else
        question_hash = {review1: question}
      total_score = scores[:total_score]
        score_map = {max: 100, min: 100, avg: 100}
      if total_score > 100
        allow(AssignmentQuestionnaire).to receive(:find_by).and_return(assignment_questionnaire2)
        total_score = 100
        allow(review_questionnaire).to receive(:get_assessments_round_for).with(any_args).and_return([response])
        allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
        participant.assignment_questionnaires(question_hash, scores)
        expect(scores[:review1][:assessments]).to eq([response])
        expect(scores[:review1][:scores]).to eq(score_map)
       end
       end
      scores[:total_score] = total_score
    scores
     end
     end
   end
   end
||
* scores(questions) method


   def scores(questions)
   describe '#merge_scores' do
     scores = {}
     context 'when all of the review_n are nil' do
    scores[:participant] = self
      it 'set max, min, avg of review score as 0' do
 
        scores = {}
    assignment_questionnaires(questions, scores)
        allow(assignment).to receive(:num_review_rounds).and_return(1)
 
        participant.merge_scores(scores)
    scores[:total_score] = self.assignment.compute_total_score(scores)
        expect(scores[:review][:scores][:max]).to eq(0)
 
        expect(scores[:review][:scores][:min]).to eq(0)
    merge_scores(scores)
         expect(scores[:review][:scores][:min]).to eq(0)
 
    # 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
     end
     end


     # for all quiz questionnaires (quizzes) taken by the participant
     context 'when the review_n is not nil' do
    quiz_responses = Array.new
      it 'merge the score of review_n to the score of review' do
    quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
        score_map = {max: 100, min: 100, avg: 100}
    quiz_response_mappings.each do |qmapping|
        scores = {review1: {scores: score_map, assessments: [response]}}
      if (qmapping.response)
        allow(assignment).to receive(:num_review_rounds).and_return(1)
         quiz_responses << qmapping.response
        participant.merge_scores(scores)
        expect(scores[:review][:scores][:max]).to eq(100)
        expect(scores[:review][:scores][:min]).to eq(100)
         expect(scores[:review][:scores][:min]).to eq(100)
       end
       end
     end
     end
    #scores[:quiz] = Hash.new
  end
    #scores[:quiz][:assessments] = quiz_responses
    #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])


     scores[:total_score] = assignment.compute_total_score(scores)
  describe '#topic_total_scores' do
    #scores[:total_score] += compute_quiz_scores(scores)
     it 'set total_score and max_pts_available of score when topic is not nil' do
 
      scores = {total_score: 100}
    calculate_scores(scores)
      allow(SignUpTopic).to receive(:find_by).with(any_args).and_return(topic)
      participant.topic_total_scores(scores)
      expect(scores[:total_score]).to eq(0)
      expect(scores[:max_pts_available]).to eq(0)
    end
   end
   end
* assignment_questionnaires(questions, scores) method


   def assignment_questionnaires(questions, scores)
   describe '#caculate_scores' do
     self.assignment.questionnaires.each do |questionnaire|
     context 'when the participant has the grade' do
       round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
       it 'his total scores equals his grade' do
       #create symbol for "varying rubrics" feature -Yang
        scores = {}
       if(round!=nil)
        expect(participant2.caculate_scores(scores)).to eq(100.0)
         questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
       end
      else
    end
        questionnaire_symbol = questionnaire.symbol
    context 'when the participant has the grade and the total score more than 100' do
       it 'return the score of a given participant with total score 100' do
        scores = {total_score: 110}
         expect(participant.caculate_scores(scores)).to eq(total_score: 100)
       end
       end
 
    end
       scores[questionnaire_symbol] = {}
    context 'when the participant has the grade and the total score less than 100' do
 
       it 'return the score of a given participant with total score' do
      if round==nil
        scores = {total_score: 90}
         scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
         expect(participant.caculate_scores(scores)).to eq(total_score: 90)
      else
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
       end
       end
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
     end
     end
   end
   end


* merge_scores(scores) method
  describe '#copy' do
    it 'copies assignment participants to a certain course' do
      expect(participant.copy(517)).to be_an_instance_of(CourseParticipant)
    end
  end


   def merge_scores(scores)
   describe '#feedback' do
    #merge scores[review#] (for each round) to score[review]  -Yang
     it 'returns corrsponding author feedback responses given by current participant' do
     if self.assignment.varying_rubrics_by_round?
       expect(participant.feedback).to eq([response])
      review_sym = "review".to_sym
    end
      scores[review_sym] = Hash.new
  end
      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]
  describe '#reviews' do
    it 'returns corrsponding peer review responses given by current team' do
      expect(participant.reviews).to eq([response])
    end
  end


        if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
  describe '#reviews_by_reviewer' do
          scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
    it 'returns corrsponding peer review responses given by certain reviewer' do
        end
      allow(ReviewResponseMap).to receive(:get_reviewer_assessments_for).with(team, participant2).and_return([response])
        if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
      expect(participant.reviews_by_reviewer(participant2)).to eq([response])
          scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
    end
        end
  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
  describe '#quizzes_taken' do
        scores[review_sym][:scores][:max] = 0
    it 'returns corrsponding quiz responses given by current participant' do
        scores[review_sym][:scores][:min] = 0
       expect(participant.quizzes_taken).to eq([response])
      end
    end
  end


       scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
  describe '#metareviews' do
    it 'returns corrsponding metareview responses given by current participant' do
       expect(participant.metareviews).to eq([response])
     end
     end
   end
   end


* calculate_scores(scores) method
  describe '#teammate_reviews' do
    it 'returns corrsponding teammate review responses given by current participant' do
      expect(participant.teammate_reviews).to eq([response])
    end
  end


   def calculate_scores(scores)
   describe '#bookmark_reviews' do
     # move lots of calculation from view(_participant.html.erb) to model
     it 'returns corrsponding bookmark review responses given by current participant' do
    if self.grade
      expect(participant.bookmark_reviews).to eq([response])
      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
   end
   end


|-
  describe '#files' do
|}
    context 'when there is not subdirectory in current directory' do
 
      it 'returns all files in current directory' do
===set_handle method===
        allow(Dir).to receive(:[]).with("a/*").and_return(["a/k.rb"])
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.
        allow(File).to receive(:directory?).with("a/k.rb").and_return(false)
        expect(participant.files("a")).to eq(["a/k.rb"])
      end
    end


{| class = "wikitable"
    context 'when there is subdirectory in current directory' do
|-
      it 'recursively returns all files in current directory' do
!Before!!After
        allow(Dir).to receive(:[]).with("a/*").and_return(["a/b"])
|-
         allow(File).to receive(:directory?).with("a/b").and_return(true)
|def set_handle
        allow(Dir).to receive(:[]).with("a/b/*").and_return(["a/b/k.rb"])
      if self.user.handle == nil or self.user.handle == ""
         allow(File).to receive(:directory?).with("a/b/k.rb").and_return(false)
         self.handle = self.user.name
         expect(participant.files("a")).to eq(["a/b/k.rb", "a/b"])
      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
       end
      self.save!
    end
   end
   end


||def set_handle
  describe ".import" do
       if self.user.handle == nil or self.user.handle == "" or AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
    context 'when record is empty' do
         self.handle = self.user.name
       it 'raises an ArgumentError' do
      else
        row = []
         self.handle = self.user.handle
        allow(AssignmentParticipant).to receive(:check_info_and_create).with(any_args).and_raise("No user id has been specified.")
         expect(AssignmentParticipant).to receive(:check_info_and_create)
         expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("No user id has been specified.")
       end
       end
      self.save!
    end
  end


|-
    context 'when no user is found by offered username' do
|}
      context 'when the record has less than 4 items' do
        it 'raises an ArgumentError' do
          row = ["user_name", "user_fullname", "name@email.com"]
          allow(AssignmentParticipant).
            to receive(:check_info_and_create).with(any_args).and_raise("The record containing #{row[0]} does not have enough items.")
          expect(AssignmentParticipant).to receive(:check_info_and_create)
          expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("The record containing #{row[0]} does not have enough items.")
        end
      end


===File and directory methods===
      context 'when the record has more than 4 items' do
The files and directory methods from AssignmentParticipant model have been moved to the '''FileHelper''' module.
        context 'when certain assignment cannot be found' do
The methods '''files''', '''submitted_files''' and '''dir_path''' are appropriate for the '''FileHelper' module and hence been moved there.
          it 'creates a new user based on import information and raises an ImportError' do
The below line was added to the AssignmentParticipant model.
            row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
include FileHelper
            session = {user: participant}
            allow(User).to receive(:find_by).with(any_args).and_return(nil)
            allow(Assignment).to receive(:find).with(2).and_return(nil)
            expect { AssignmentParticipant.import(row, nil, session, 2) }.
              to raise_error("The assignment with id \"2\" was not found.").and change { User.count }.by(1)
          end
        end


The AssignmentParticipant class would look like
        context 'when certain assignment can be found and assignment participant does not exists' do
  class AssignmentParticipant < Participant
          it 'creates a new user, new participant and raises an ImportError' do
    require 'wiki_helper'
            row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
    '''include FileHelper'''
            session = {user: participant}
Methods moved to FileHelper:
            allow(User).to receive(:find_by_name).with(any_args).and_return(nil)
            allow(Assignment).to receive(:find).with(2).and_return(assignment)
            allow(AssignmentParticipant).to receive(:exists?).and_return(false)
            expect { AssignmentParticipant.import(row, nil, session, 2) }.to change { User.count }.by(1).and change { AssignmentParticipant.count }.by(1)
          end
        end
      end
    end
  end


* submitted_files
  describe '.export' do
 
    it 'exports all participants in current assignment' do
  def submitted_files
      csv = []
    files(self.path) if self.directory_num
      expect(AssignmentParticipant).to receive_message_chain(:where, :find_each).with(any_args).and_yield(participant)
      expect(AssignmentParticipant.export(csv, 1, any_args)).
        to eq([["student2064", "2064, student", "expertiza@mailinator.com", "Student", "instructor6", true, true, true, "handle"]])
    end
   end
   end


* files(directory)
  describe '#set_handle' do
    context 'when the user of current participant does not have handle' do
      it 'sets the user name as the handle of current participant' do
        allow(student).to receive_message_chain(:handle, :nil?).and_return(true)
        allow(student).to receive(:handle).and_return("")
        participant.set_handle
        expect(participant.handle).to eq("student2064")
      end
    end


  def files(directory)
    context 'when current assignment exists participants with same handle as the one of current user' do
    files_list = Dir[directory + "/*"]
      it 'sets the user name as the name of current participant' do
     files = Array.new
        allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
        participant.set_handle
        expect(participant.handle).to eq("student2064")
      end
     end


     files_list.each do |file|
     context 'when current assignment does not have participants with same handle as the one of current user' do
       if File.directory?(file)
       it 'sets the user name as the handle of current participant' do
         dir_files = files(file)
        participant.set_handle
        dir_files.each{|f| files << f}
         expect(participant.handle).to eq("handle")
       end
       end
      files << file
     end
     end
    files
   end
   end


* submitted_files()
  describe '#review_file_path' do
 
    it 'returns the file path for reviewer to upload files during peer review' do
  def submitted_files()
      allow(ResponseMap).to receive(:find).with(any_args).and_return(response_map)
    files = Array.new
      allow(TeamsUser).to receive_message_chain(:find_by, :user_id).with(any_args).and_return(1)
    if(self.directory_num)
      allow(Participant).to receive(:find_by).with(any_args).and_return(participant)
       files = files(self.path)
       file_path = Rails.root.to_s + "/pg_data/instructor6/csc517/test/final_test/0_review/1"
      expect(participant.review_file_path(1)).to eq(file_path)
     end
     end
    return files
   end
   end


* dir_path
   describe '#current_stage' do
 
     it 'returns stage of current assignment' do
   def dir_path
      allow(assignment).to receive(:get_current_stage).with(1).and_return("Finished")
     assignment.try :directory_path
      expect(participant.current_stage).to eq("Finished")
    end
   end
   end


===Remove unused methods===
  describe '#stage_deadline' do
The following methods have been removed as they were not being used by the application.
    context 'when stage of current assignment is not Finished' do
      it 'returns current stage' do
        allow(assignment).to receive(:stage_deadline).with(1).and_return("Unknow")
        expect(participant.stage_deadline).to eq("Unknow")
      end
    end


'''def average_score_per_assignment(assignment_id)'''
    context 'when stage of current assignment not Finished' do
      context 'current assignment is not a staggered deadline assignment' do
        it 'returns the due date of current assignment' do
          allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
          allow(assignment).to receive(:staggered_deadline?).and_return(false)
          allow(assignment).to receive_message_chain(:due_dates, :last, :due_at).and_return(1)
          expect(participant.stage_deadline).to eq("1")
        end
      end


def average_score_per_assignment(assignment_id)
      context 'current assignment is a staggered deadline assignment' do
    return 0 if self.response_maps.size == 0
        it 'returns the due date of current topic' do
 
          allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
    sum_of_scores = 0
          allow(assignment).to receive(:staggered_deadline?).and_return(true)
 
          allow(TopicDueDate).to receive_message_chain(:find_by, :last, :due_at).and_return(1)
    self.response_maps.metareview_response_maps.each do |metaresponse_map|
          expect(participant.stage_deadline).to eq("1")
      if !metaresponse_map.response.empty? && response_map == assignment_id then
         end
         sum_of_scores = sum_of_scores + response_map.response.last.average_score
       end
       end
     end
     end
    (sum_of_scores / self.response_maps.size).to_i
   end
   end


== Refactoring ==
===Scores method===
The method '''scores''' has been converted to smaller methods '''scores''', '''assignment_questionnaires''',  '''merge_scores''',  '''topic_total_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.


'''def quiz_taken_by?(contributor, reviewer)'''
{| class = "wikitable"
|-
!Before!!After
|-
def scores(questions)
    scores = {}
    scores[:participant] = self
    self.assignment.questionnaires.each do |questionnaire|
      round = AssignmentQuestionnaire.find_by(assignment_id: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
      # create symbol for "varying rubrics" feature -Yang
      questionnaire_symbol = if round.nil?
                              questionnaire.symbol
                            else
                              (questionnaire.symbol.to_s + round.to_s).to_sym
                            end


  def quiz_taken_by?(contributor, reviewer)
      scores[questionnaire_symbol] = {}
    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


      scores[questionnaire_symbol][:assessments] = if round.nil?
                                                    questionnaire.get_assessments_for(self)
                                                  else
                                                    questionnaire.get_assessments_round_for(self, round)
                                                  end
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
    end


'''def has_quiz?'''
    scores[:total_score] = self.assignment.compute_total_score(scores)


  def has_quiz?
    # merge scores[review#] (for each round) to score[review]  -Yang
     return !QuizQuestionnaire.find_by_instructor_id(self.id).nil?
    if self.assignment.varying_rubrics_by_round?
      review_sym = "review".to_sym
      scores[review_sym] = {}
      scores[review_sym][:assessments] = []
      scores[review_sym][:scores] = {}
      scores[review_sym][:scores][:max] = -999_999_999
      scores[review_sym][:scores][:min] = 999_999_999
      scores[review_sym][:scores][:avg] = 0
      total_score = 0
      for i in 1..self.assignment.num_review_rounds
        round_sym = ("review" + i.to_s).to_sym
        if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
          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
        unless scores[round_sym][:scores][:avg].nil?
          total_score += scores[round_sym][:scores][:avg] * length_of_assessments
        end
      end
      if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
        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)
      unless 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 = []
    # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
    # quiz_response_mappings.each do |qmapping|
    #  quiz_responses << qmapping.response if qmapping.response
    # 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
      scores[:total_score] = 100 if scores[:total_score] > 100
      scores
    end
   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[review#] (for each round) to score[review]  -Yang
    merge_scores(scores) if self.assignment.varying_rubrics_by_round?
    # 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
    topic_total_scores(scores) if self.assignment.is_microtask?


'''def reviewees'''
    # for all quiz questionnaires (quizzes) taken by the participant
    # quiz_responses = []
    # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
    # quiz_response_mappings.each do |qmapping|
    #  quiz_responses << qmapping.response if qmapping.response
    # 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
    caculate_scores(scores)
  end


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


     reviewees
  def assignment_questionnaires(questions, scores)
  end
     self.assignment.questionnaires.each do |questionnaire|
      round = AssignmentQuestionnaire.find_by(assignment_id: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
      # create symbol for "varying rubrics" feature -Yang
      questionnaire_symbol = if round.nil?
                              questionnaire.symbol
                            else
                              (questionnaire.symbol.to_s + round.to_s).to_sym
                            end


      scores[questionnaire_symbol] = {}


'''def two_node_cycles'''
      scores[questionnaire_symbol][:assessments] = if round.nil?
                                                    questionnaire.get_assessments_for(self)
  def two_node_cycles
                                                  else
    cycles = []
                                                    questionnaire.get_assessments_round_for(self, round)
    self.reviewers.each do |ap|
                                                  end
      if ap.reviewers.include?(self)
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
        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
     end
    cycles
   end
   end


* merge_scores(scores) method


'''def three_node_cycles'''
  def merge_scores(scores)
 
    review_sym = "review".to_sym
  def three_node_cycles
    scores[review_sym] = {}
     cycles = []
     scores[review_sym][:assessments] = []
     self.reviewers.each do |ap1|
     scores[review_sym][:scores] = {max: -999_999_999, min: 999_999_999, avg: 0}
       ap1.reviewers.each do |ap2|
    total_score = 0
        if ap2.reviewers.include?(self)
    for i in 1..self.assignment.num_review_rounds
          self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
       round_sym = ("review" + i.to_s).to_sym
          ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
      if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
          ap2.reviews_by_reviewer(self).nil? ? next : s20 = ap2.reviews_by_reviewer(self).get_total_score
        next
          cycles.push([[self, s01], [ap1, s12], [ap2, s20]])
      end
         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
      unless scores[round_sym][:scores][:avg].nil?
         total_score += scores[round_sym][:scores][:avg] * length_of_assessments
       end
       end
     end
     end
     cycles
     if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
      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


*  topic_total_scores(scores) method


'''def four_node_cycles'''
   def topic_total_scores(scores)
 
     topic = SignUpTopic.find_by(assignment_id: self.assignment.id)
   def four_node_cycles
    unless topic.nil?
     cycles = []
      scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
    self.reviewers.each do |ap1|
      scores[:max_pts_available] = topic.micropayment
      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
     end
    cycles
   end
   end


* calculate_scores(scores) method


'''def cycle_similarity_score(cycle)'''
   def caculate_scores(scores)
 
     if self.grade
   def cycle_similarity_score(cycle)
      scores[:total_score] = self.grade
     similarity_score = 0.0
     else
    count = 0.0
       scores[:total_score] = 100 if scores[:total_score] > 100
 
       scores
     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
     end
    similarity_score = similarity_score / count unless count == 0.0
    similarity_score
   end
   end


|-
|}


'''def cycle_deviation_score(cycle)'''
===Files method===
The files method of AssignmentParticipant model is exactly the same as '''assignment_team.rb, L103''', so we move it to a module named Instance_method in '''lib/TFD1770_refactor.rb'''.
The below two lines are added to the AssignmentParticipant model and AssignmentTeam model.


  def cycle_deviation_score(cycle)
require 'TFD1770_refactor'
    deviation_score = 0.0
    count = 0.0


    0 ... cycle.size.each do |member|
include Instance_method
      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
* module Instance_method
     deviation_score
  def files(directory)
  end
     files_list = Dir[directory + "/*"]
     files = []


 
    files_list.each do |file|
'''def compute_quiz_scores(scores)'''
      if File.directory?(file)
 
        dir_files = files(file)
  def compute_quiz_scores(scores)
        dir_files.each {|f| files << f }
    total = 0
       end
    if scores[:quiz][:scores][:avg]
       files << file
       return scores[:quiz][:scores][:avg] * 100  / 100.to_f
    else
       return 0
     end
     end
     return total
     files
   end
   end


===Self.import  method===
The self.import method of AssignmentParticipant model is exactly the same as '''course_participant.rb, L21''', so we move it to a module named Class_method in '''lib/TFD1770_refactor.rb'''.
The below two lines are added to the AssignmentParticipant model and CourseParticipant model.


'''def review_response_maps'''
require 'TFD1770_refactor'


  def review_response_maps
include Class_method
      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 self.import(row, _row_header = nil, session, id)
'''def members'''
    user = AssignmentParticipant.check_info_and_create(row, _row_header = nil, session)
 
    raise ImportError, "The assignment with id \"" + id.to_s + "\" was not found." if Assignment.find(id).nil?
  def members
     unless AssignmentParticipant.exists?(user_id: user.id, parent_id: id)
     team.try :participants
      new_part = AssignmentParticipant.create(user_id: user.id, parent_id: id)
      new_part.set_handle
    end
   end
   end


 
* module Class_method
'''def get_hash(time_stamp)'''
   def check_info_and_create(row, _row_header = nil, session)
 
     raise ArgumentError, "No user id has been specified." if row.empty?
   def get_hash(time_stamp)
     user = User.find_by(name: row[0])
     # first generate a hash from the assignment name itself
     if user.nil?
     hash_data = Digest::SHA1.digest(self.assignment.name.to_s)
      raise ArgumentError, "The record containing #{row[0]} does not have enough items." if row.length < 4
 
      attributes = ImportFileHelper.define_attributes(row)
     # second generate a hash from the first hash plus the user name and time stamp
      user = ImportFileHelper.create_new_user(attributes, session)
    sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S")
    end
    Digest::SHA1.digest(sign)
   end
   end


===Review methods===
===Find_by method===
* 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.
Use '''find_by''' instead of dynamic method


{| class = "wikitable"
{| class = "wikitable"
Line 533: Line 671:
!Before!!After
!Before!!After
|-
|-
|def bookmark_reviews
|
    BookmarkRatingResponseMap.get_assessments_for(self)
*  assignment_particpant.rb L32
  end
quiz = QuizQuestionnaire.find_by_instructor_id(contributor.id)
 
* assignment_particpant.rb L113
topic = SignUpTopic.find_by_assignment_id(assignment.id)
 
* assignment_particpant.rb L196
user = User.find_by_name(row[0])
 
|| 
*  assignment_particpant.rb L34
quiz = QuizQuestionnaire.find_by(instructor_id: contributor.id)


||def get_assessments_for(participant)
* assignment_particpant.rb L134
    BookmarkRatingResponseMap.get_assessments_for(participant) # participant.bookmark_reviews()
topic = SignUpTopic.find_by(assignment_id: self.assignment.id)
  end


* TFD1770_refactor.rb L20
user = User.find_by(name:row[0])
|-
|-
|}
|}


* 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.
Use '''find_by''' instead of where.first


{| class = "wikitable"
{| class = "wikitable"
Line 550: Line 699:
!Before!!After
!Before!!After
|-
|-
|def teammate_reviews
|
    TeammateReviewResponseMap.get_assessments_for(self)
*  assignment_particpant.rb L267
  end
first_user_id = TeamsUser.where(team_id: response_map.reviewee_id).first.user_id
 
* assignment_particpant.rb L267
participant = Participant.where(parent_id: response_map.reviewed_object_id, user_id: first_user_id).first
 
||
*  assignment_particpant.rb L258
first_user_id = TeamsUser.find_by(team_id: response_map.reviewee_id).user_id


||def get_assessments_for(participant)
* assignment_particpant.rb L259
    TeammateReviewResponseMap.get_assessments_for(participant)  #participant.teammate_reviews()
participant = Participant.find_by(parent_id: response_map.reviewed_object_id, user_id: first_user_id)
  end


|-
|-
Line 563: Line 718:
==Resources==
==Resources==
* Expertiza Github repository<ref>Expertiza Github repository https://github.com/expertiza/expertiza</ref>
* Expertiza Github repository<ref>Expertiza Github repository https://github.com/expertiza/expertiza</ref>
* Our Github repository<ref>Our Github Repository https://github.com/terryliu1995/expertiza</ref>
* Our Github repository<ref>Our Github repository https://github.com/terryliu1995/expertiza</ref>
* Our pull request<ref>Our pull request https://github.com/expertiza/expertiza/pull/1049</ref>


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

Latest revision as of 12:52, 3 November 2017

E1770. [Test First Development] Refactor assignment_participant.rb<ref>Project Description document https://docs.google.com/document/d/1rdolBAHxVGI9I0N-cT866AqnfORM2L1_m_bo2gRYRrI/edit#heading=h.s18ldps2kz5e</ref>

This page provides a brief description of the Expertiza project. The project is aimed at using TFD(Test First Development) to refactor 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.

We used the TDF(Test First Development) method to refactor the AssignmentParticipant model. With the RSpec tool, we wrote 38 test cases for 24 methods in assignment_participant_spec.rb first. After finishing these test cases, we executed them, and most of them failed. And then we refactored the methods, scores, files, self.import, find_by, where.first, and executed the test cases again, finally they all passed.

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?

Code Refactoring<ref>Refactoring https://en.wikipedia.org/wiki/Code_refactoring</ref> is the process of restructuring existing computer code—changing the factoring—without changing its external behavior.

Refactoring improves nonfunctional attributes of the software. Advantages include improved code readability and reduced complexity; these can improve source-code maintainability and create a more expressive internal architecture or object model to improve extensibility. Typically, refactoring applies a series of standardised basic micro-refactorings, each of which is (usually) a tiny change in a computer program's source code that either preserves the behaviour of the software, or at least does not modify its conformance to functional requirements. Many development environments provide automated support for performing the mechanical aspects of these basic refactorings. If done extremely well, code refactoring may also resolve hidden, dormant, or undiscovered bugs or vulnerabilities in the system by simplifying the underlying logic and eliminating unnecessary levels of complexity. If done poorly it may fail the requirement that external functionality not be changed, introduce new bugs, or both.

Why Test-driven development?

Test-driven development (TDD)<ref>Test-driven development (TDD) https://en.wikipedia.org/wiki/Test-driven_development</ref> is a software development process that relies on the repetition of a very short development cycle: Requirements are turned into very specific test cases, then the software is improved to pass the new tests, only. This is opposed to software development that allows software to be added that is not proven to meet requirements. The followings are several benefits of Test-driven development (TDD).

  • Maintainable, Flexible, Easily Extensible.
  • Unparalleled Test Coverage & Streamlined Codebase.
  • Clean Interface.
  • Refactoring Encourages Improvements.
  • Executable Documentation.

Introduction to RSpec

RSpec<ref>RSpec https://en.wikipedia.org/wiki/RSpec</ref> is a 'Domain Specific Language' (DSL) testing tool written in Ruby to test Ruby code. It is a behavior-driven development (BDD) framework which is extensively used in the production applications. The basic idea behind this concept is that of Test Driven Development(TDD) where the tests are written first and the development is based on writing just enough code that will fulfill those tests followed by refactoring. It contains its own mocking framework that is fully integrated into the framework based upon JMock. The simplicity in the RSpec syntax makes it one of the popular testing tools for Ruby applications. The RSpec tool can be used by installing the rspec gem which consists of 3 other gems namely rspec-core , rspec-expectation and rspec-mock.

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.
    • Write failing tests first.
    • Split into several simpler methods and assign reasonable names.
    • Extract duplicated code into separate methods.
    • Replace the conditional with the relevant method calls.
  • Method files is exactly the same as assignment_team.rb L103.
    • Write failing tests first.
    • Solve the duplication, extract method to a new file or delete useless one.
  • Method self.import is exact the same as course_participant.rb L21.
    • Write failing tests first.
    • Solve the duplication, extract method to a new file or delete useless one.
  • Use find_by instead of dynamic method.
    • Write failing tests first.
  • Use find_by instead of where.first.
    • Write failing tests first.

Test Cases

Because the project is aimed at using TFD(Test First Development) to refactor the AssignmentParticipant model. We wrote 38 test cases for 24 methods in assignment_participant_spec.rb first, with the RSpec tool.

The introduction video to our project<ref>Introduction video to our project https://www.youtube.com/watch?v=sJdmN-S2Voo&feature=youtu.be</ref>

 describe '#dir_path' do
   it 'returns the directory path of current assignment' do
     expect(participant.dir_path).to eq "final_test"
   end
 end
 describe '#assign_quiz' do
   it 'creates a new QuizResponseMap record' do
     allow(QuizQuestionnaire).to receive(:find_by).with(instructor_id: 1).and_return(quiz_questionaire) # WHY CAN NOT DELETE THIS SENTENCE
     expect { participant.assign_quiz(participant, participant2, nil) }.to change { QuizResponseMap.count }.from(0).to(1)
     expect(participant.assign_quiz(participant, participant2, nil)).to be_an_instance_of(QuizResponseMap)
   end
 end
 describe '#reviewers' do
   it 'returns all the participants in this assignment who have reviewed the team where this participant belongs' do
     allow(ReviewResponseMap).to receive(:where).with(any_args).and_return([response_map]) # differences with .with(parameter)
     allow(AssignmentParticipant).to receive(:find).with(any_args).and_return(participant2)
     expect(participant.reviewers).to eq([participant2])
   end
 end
 describe '#review_score' do
   it 'returns the review score' do
     # diao yong de method tai duo le,er qie bu zhi dao sha yi si
     allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
     allow(review_questionnaire).to receive(:questions).and_return(question)
     allow(Answer).to receive(:compute_scores).with([response], question).and_return(avg: 100)
     allow(review_questionnaire).to receive(:max_possible_score).and_return(100)
     expect(participant.review_score).to eq(100)
   end
 end
 describe '#scores' do
   context 'when assignment is not varying rubric by round and not an microtask' do
     it 'calculates scores that this participant has been given' do
       expect(participant).to receive(:assignment_questionnaires)
       expect(assignment).to receive(:compute_total_score)
       allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
       allow(assignment).to receive(:is_microtask?).and_return(false)
       expect(assignment).to receive(:compute_total_score)
       expect(participant).to receive(:caculate_scores)
       participant.scores(question)
     end
   end
   context 'when assignment is varying rubric by round but not an microtask' do
     it 'calculates scores that this participant has been given' do
       expect(participant).to receive(:assignment_questionnaires)
       expect(assignment).to receive(:compute_total_score)
       allow(assignment).to receive(:varying_rubrics_by_round?).and_return(true)
       expect(participant).to receive(:merge_scores)
       allow(assignment).to receive(:is_microtask?).and_return(false)
       expect(assignment).to receive(:compute_total_score)
       expect(participant).to receive(:caculate_scores)
       participant.scores(question)
     end
   end
   context 'when assignment is not varying rubric by round but an microtask' do
     it 'calculates scores that this participant has been given' do
       expect(participant).to receive(:assignment_questionnaires)
       expect(assignment).to receive(:compute_total_score)
       allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
       allow(assignment).to receive(:is_microtask?).and_return(true)
       expect(participant).to receive(:topic_total_scores)
       expect(assignment).to receive(:compute_total_score)
       expect(participant).to receive(:caculate_scores)
       participant.scores(question)
     end
   end
 end
 # included method
 describe '#assignment_questionnaires' do
   context 'when the round of questionnaire is nil' do
     it 'record the result as review scores' do
       scores = {}
       question_hash = {review: question}
       score_map = {max: 100, min: 100, avg: 100}
       allow(AssignmentQuestionnaire).to receive(:find_by).with(any_args).and_return(assignment_questionnaire)
       allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
       allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
       participant.assignment_questionnaires(question_hash, scores)
       expect(scores[:review][:assessments]).to eq([response])
       expect(scores[:review][:scores]).to eq(score_map)
     end
   end
   context 'when the round of questionnaire is not nil' do
     it 'record the result as review#{n} scores' do
       scores = {}
       question_hash = {review1: question}
       score_map = {max: 100, min: 100, avg: 100}
       allow(AssignmentQuestionnaire).to receive(:find_by).and_return(assignment_questionnaire2)
       allow(review_questionnaire).to receive(:get_assessments_round_for).with(any_args).and_return([response])
       allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
       participant.assignment_questionnaires(question_hash, scores)
       expect(scores[:review1][:assessments]).to eq([response])
       expect(scores[:review1][:scores]).to eq(score_map)
     end
   end
 end
 describe '#merge_scores' do
   context 'when all of the review_n are nil' do
     it 'set max, min, avg of review score as 0' do
       scores = {}
       allow(assignment).to receive(:num_review_rounds).and_return(1)
       participant.merge_scores(scores)
       expect(scores[:review][:scores][:max]).to eq(0)
       expect(scores[:review][:scores][:min]).to eq(0)
       expect(scores[:review][:scores][:min]).to eq(0)
     end
   end
   context 'when the review_n is not nil' do
     it 'merge the score of review_n to the score of review' do
       score_map = {max: 100, min: 100, avg: 100}
       scores = {review1: {scores: score_map, assessments: [response]}}
       allow(assignment).to receive(:num_review_rounds).and_return(1)
       participant.merge_scores(scores)
       expect(scores[:review][:scores][:max]).to eq(100)
       expect(scores[:review][:scores][:min]).to eq(100)
       expect(scores[:review][:scores][:min]).to eq(100)
     end
   end
 end
 describe '#topic_total_scores' do
   it 'set total_score and max_pts_available of score when topic is not nil' do
     scores = {total_score: 100}
     allow(SignUpTopic).to receive(:find_by).with(any_args).and_return(topic)
     participant.topic_total_scores(scores)
     expect(scores[:total_score]).to eq(0)
     expect(scores[:max_pts_available]).to eq(0)
   end
 end
 describe '#caculate_scores' do
   context 'when the participant has the grade' do
     it 'his total scores equals his grade' do
       scores = {}
       expect(participant2.caculate_scores(scores)).to eq(100.0)
     end
   end
   context 'when the participant has the grade and the total score more than 100' do
     it 'return the score of a given participant with total score 100' do
       scores = {total_score: 110}
       expect(participant.caculate_scores(scores)).to eq(total_score: 100)
     end
   end
   context 'when the participant has the grade and the total score less than 100' do
     it 'return the score of a given participant with total score' do
       scores = {total_score: 90}
       expect(participant.caculate_scores(scores)).to eq(total_score: 90)
     end
   end
 end
 describe '#copy' do
   it 'copies assignment participants to a certain course' do
     expect(participant.copy(517)).to be_an_instance_of(CourseParticipant)
   end
 end
 describe '#feedback' do
   it 'returns corrsponding author feedback responses given by current participant' do
     expect(participant.feedback).to eq([response])
   end
 end
 describe '#reviews' do
   it 'returns corrsponding peer review responses given by current team' do
     expect(participant.reviews).to eq([response])
   end
 end
 describe '#reviews_by_reviewer' do
   it 'returns corrsponding peer review responses given by certain reviewer' do
     allow(ReviewResponseMap).to receive(:get_reviewer_assessments_for).with(team, participant2).and_return([response])
     expect(participant.reviews_by_reviewer(participant2)).to eq([response])
   end
 end
 describe '#quizzes_taken' do
   it 'returns corrsponding quiz responses given by current participant' do
     expect(participant.quizzes_taken).to eq([response])
   end
 end
 describe '#metareviews' do
   it 'returns corrsponding metareview responses given by current participant' do
     expect(participant.metareviews).to eq([response])
   end
 end
 describe '#teammate_reviews' do
   it 'returns corrsponding teammate review responses given by current participant' do
     expect(participant.teammate_reviews).to eq([response])
   end
 end
 describe '#bookmark_reviews' do
   it 'returns corrsponding bookmark review responses given by current participant' do
     expect(participant.bookmark_reviews).to eq([response])
   end
 end
 describe '#files' do
   context 'when there is not subdirectory in current directory' do
     it 'returns all files in current directory' do
       allow(Dir).to receive(:[]).with("a/*").and_return(["a/k.rb"])
       allow(File).to receive(:directory?).with("a/k.rb").and_return(false)
       expect(participant.files("a")).to eq(["a/k.rb"])
     end
   end
   context 'when there is subdirectory in current directory' do
     it 'recursively returns all files in current directory' do
       allow(Dir).to receive(:[]).with("a/*").and_return(["a/b"])
       allow(File).to receive(:directory?).with("a/b").and_return(true)
       allow(Dir).to receive(:[]).with("a/b/*").and_return(["a/b/k.rb"])
       allow(File).to receive(:directory?).with("a/b/k.rb").and_return(false)
       expect(participant.files("a")).to eq(["a/b/k.rb", "a/b"])
     end
   end
 end
 describe ".import" do
   context 'when record is empty' do
     it 'raises an ArgumentError' do
       row = []
       allow(AssignmentParticipant).to receive(:check_info_and_create).with(any_args).and_raise("No user id has been specified.")
       expect(AssignmentParticipant).to receive(:check_info_and_create)
       expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("No user id has been specified.")
     end
   end
   context 'when no user is found by offered username' do
     context 'when the record has less than 4 items' do
       it 'raises an ArgumentError' do
         row = ["user_name", "user_fullname", "name@email.com"]
         allow(AssignmentParticipant).
           to receive(:check_info_and_create).with(any_args).and_raise("The record containing #{row[0]} does not have enough items.")
         expect(AssignmentParticipant).to receive(:check_info_and_create)
         expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("The record containing #{row[0]} does not have enough items.")
       end
     end
     context 'when the record has more than 4 items' do
       context 'when certain assignment cannot be found' do
         it 'creates a new user based on import information and raises an ImportError' do
           row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
           session = {user: participant}
           allow(User).to receive(:find_by).with(any_args).and_return(nil)
           allow(Assignment).to receive(:find).with(2).and_return(nil)
           expect { AssignmentParticipant.import(row, nil, session, 2) }.
             to raise_error("The assignment with id \"2\" was not found.").and change { User.count }.by(1)
         end
       end
       context 'when certain assignment can be found and assignment participant does not exists' do
         it 'creates a new user, new participant and raises an ImportError' do
           row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
           session = {user: participant}
           allow(User).to receive(:find_by_name).with(any_args).and_return(nil)
           allow(Assignment).to receive(:find).with(2).and_return(assignment)
           allow(AssignmentParticipant).to receive(:exists?).and_return(false)
           expect { AssignmentParticipant.import(row, nil, session, 2) }.to change { User.count }.by(1).and change { AssignmentParticipant.count }.by(1)
         end
       end
     end
   end
 end
 describe '.export' do
   it 'exports all participants in current assignment' do
     csv = []
     expect(AssignmentParticipant).to receive_message_chain(:where, :find_each).with(any_args).and_yield(participant)
     expect(AssignmentParticipant.export(csv, 1, any_args)).
       to eq("student2064", "2064, student", "expertiza@mailinator.com", "Student", "instructor6", true, true, true, "handle")
   end
 end
 describe '#set_handle' do
   context 'when the user of current participant does not have handle' do
     it 'sets the user name as the handle of current participant' do
       allow(student).to receive_message_chain(:handle, :nil?).and_return(true)
       allow(student).to receive(:handle).and_return("")
       participant.set_handle
       expect(participant.handle).to eq("student2064")
     end
   end
   context 'when current assignment exists participants with same handle as the one of current user' do
     it 'sets the user name as the name of current participant' do
       allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
       participant.set_handle
       expect(participant.handle).to eq("student2064")
     end
   end
   context 'when current assignment does not have participants with same handle as the one of current user' do
     it 'sets the user name as the handle of current participant' do
       participant.set_handle
       expect(participant.handle).to eq("handle")
     end
   end
 end
 describe '#review_file_path' do
   it 'returns the file path for reviewer to upload files during peer review' do
     allow(ResponseMap).to receive(:find).with(any_args).and_return(response_map)
     allow(TeamsUser).to receive_message_chain(:find_by, :user_id).with(any_args).and_return(1)
     allow(Participant).to receive(:find_by).with(any_args).and_return(participant)
     file_path = Rails.root.to_s + "/pg_data/instructor6/csc517/test/final_test/0_review/1"
     expect(participant.review_file_path(1)).to eq(file_path)
   end
 end
 describe '#current_stage' do
   it 'returns stage of current assignment' do
     allow(assignment).to receive(:get_current_stage).with(1).and_return("Finished")
     expect(participant.current_stage).to eq("Finished")
   end
 end
 describe '#stage_deadline' do
   context 'when stage of current assignment is not Finished' do
     it 'returns current stage' do
       allow(assignment).to receive(:stage_deadline).with(1).and_return("Unknow")
       expect(participant.stage_deadline).to eq("Unknow")
     end
   end
   context 'when stage of current assignment not Finished' do
     context 'current assignment is not a staggered deadline assignment' do
       it 'returns the due date of current assignment' do
         allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
         allow(assignment).to receive(:staggered_deadline?).and_return(false)
         allow(assignment).to receive_message_chain(:due_dates, :last, :due_at).and_return(1)
         expect(participant.stage_deadline).to eq("1")
       end
     end
     context 'current assignment is a staggered deadline assignment' do
       it 'returns the due date of current topic' do
         allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
         allow(assignment).to receive(:staggered_deadline?).and_return(true)
         allow(TopicDueDate).to receive_message_chain(:find_by, :last, :due_at).and_return(1)
         expect(participant.stage_deadline).to eq("1")
       end
     end
   end
 end

Refactoring

Scores method

The method scores has been converted to smaller methods scores, assignment_questionnaires, merge_scores, topic_total_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: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
     # create symbol for "varying rubrics" feature -Yang
     questionnaire_symbol = if round.nil?
                              questionnaire.symbol
                            else
                              (questionnaire.symbol.to_s + round.to_s).to_sym
                            end
     scores[questionnaire_symbol] = {}
     scores[questionnaire_symbol][:assessments] = if round.nil?
                                                    questionnaire.get_assessments_for(self)
                                                  else
                                                    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] = {}
     scores[review_sym][:assessments] = []
     scores[review_sym][:scores] = {}
     scores[review_sym][:scores][:max] = -999_999_999
     scores[review_sym][:scores][:min] = 999_999_999
     scores[review_sym][:scores][:avg] = 0
     total_score = 0
     for i in 1..self.assignment.num_review_rounds
       round_sym = ("review" + i.to_s).to_sym
       if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
         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
       unless scores[round_sym][:scores][:avg].nil?
         total_score += scores[round_sym][:scores][:avg] * length_of_assessments
       end
     end
     if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
       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)
     unless 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 = []
   # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
   # quiz_response_mappings.each do |qmapping|
   #   quiz_responses << qmapping.response if qmapping.response
   # 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
     scores[:total_score] = 100 if scores[:total_score] > 100
     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[review#] (for each round) to score[review]  -Yang
   merge_scores(scores) if self.assignment.varying_rubrics_by_round?
   # 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
   topic_total_scores(scores) if self.assignment.is_microtask?
   # for all quiz questionnaires (quizzes) taken by the participant
   # quiz_responses = []
   # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
   # quiz_response_mappings.each do |qmapping|
   #   quiz_responses << qmapping.response if qmapping.response
   # 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
   caculate_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: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
     # create symbol for "varying rubrics" feature -Yang
     questionnaire_symbol = if round.nil?
                              questionnaire.symbol
                            else
                              (questionnaire.symbol.to_s + round.to_s).to_sym
                            end
     scores[questionnaire_symbol] = {}
     scores[questionnaire_symbol][:assessments] = if round.nil?
                                                    questionnaire.get_assessments_for(self)
                                                  else
                                                    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)
   review_sym = "review".to_sym
   scores[review_sym] = {}
   scores[review_sym][:assessments] = []
   scores[review_sym][:scores] = {max: -999_999_999, min: 999_999_999, avg: 0}
   total_score = 0
   for i in 1..self.assignment.num_review_rounds
     round_sym = ("review" + i.to_s).to_sym
     if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
       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
     unless scores[round_sym][:scores][:avg].nil?
       total_score += scores[round_sym][:scores][:avg] * length_of_assessments
     end
   end
   if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
     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
  • topic_total_scores(scores) method
 def topic_total_scores(scores)
   topic = SignUpTopic.find_by(assignment_id: self.assignment.id)
   unless topic.nil?
     scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
     scores[:max_pts_available] = topic.micropayment
   end
 end
  • calculate_scores(scores) method
 def caculate_scores(scores)
   if self.grade
     scores[:total_score] = self.grade
   else
     scores[:total_score] = 100 if scores[:total_score] > 100
     scores
   end
 end

Files method

The files method of AssignmentParticipant model is exactly the same as assignment_team.rb, L103, so we move it to a module named Instance_method in lib/TFD1770_refactor.rb. The below two lines are added to the AssignmentParticipant model and AssignmentTeam model.

require 'TFD1770_refactor'

include Instance_method

  • module Instance_method
 def files(directory)
   files_list = Dir[directory + "/*"]
   files = []
   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

Self.import method

The self.import method of AssignmentParticipant model is exactly the same as course_participant.rb, L21, so we move it to a module named Class_method in lib/TFD1770_refactor.rb. The below two lines are added to the AssignmentParticipant model and CourseParticipant model.

require 'TFD1770_refactor'

include Class_method

 def self.import(row, _row_header = nil, session, id)
   user = AssignmentParticipant.check_info_and_create(row, _row_header = nil, session)
   raise ImportError, "The assignment with id \"" + id.to_s + "\" was not found." if Assignment.find(id).nil?
   unless AssignmentParticipant.exists?(user_id: user.id, parent_id: id)
     new_part = AssignmentParticipant.create(user_id: user.id, parent_id: id)
     new_part.set_handle
   end
 end
  • module Class_method
 def check_info_and_create(row, _row_header = nil, session)
   raise ArgumentError, "No user id has been specified." if row.empty?
   user = User.find_by(name: row[0])
   if user.nil?
     raise ArgumentError, "The record containing #{row[0]} does not have enough items." if row.length < 4
     attributes = ImportFileHelper.define_attributes(row)
     user = ImportFileHelper.create_new_user(attributes, session)
   end
 end

Find_by method

Use find_by instead of dynamic method

Before After
  • assignment_particpant.rb L32

quiz = QuizQuestionnaire.find_by_instructor_id(contributor.id)

  • assignment_particpant.rb L113

topic = SignUpTopic.find_by_assignment_id(assignment.id)

  • assignment_particpant.rb L196

user = User.find_by_name(row[0])

  • assignment_particpant.rb L34

quiz = QuizQuestionnaire.find_by(instructor_id: contributor.id)

  • assignment_particpant.rb L134

topic = SignUpTopic.find_by(assignment_id: self.assignment.id)

  • TFD1770_refactor.rb L20

user = User.find_by(name:row[0])

Use find_by instead of where.first

Before After
  • assignment_particpant.rb L267

first_user_id = TeamsUser.where(team_id: response_map.reviewee_id).first.user_id

  • assignment_particpant.rb L267

participant = Participant.where(parent_id: response_map.reviewed_object_id, user_id: first_user_id).first

  • assignment_particpant.rb L258

first_user_id = TeamsUser.find_by(team_id: response_map.reviewee_id).user_id

  • assignment_particpant.rb L259

participant = Participant.find_by(parent_id: response_map.reviewed_object_id, user_id: first_user_id)

Resources

References

<references/>