CSC/ECE 517 Fall 2017/E1770 Refactor assignment participant.rb

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

• 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 pull request<ref>Our pull request https://github.com/expertiza/expertiza/pull/1049</ref>

References

<references/>