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

Introduction to Expertiza

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

Why refactoring?

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.

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

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

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>

References

<references/>