CSC/ECE 517 Fall 2015/oss E1562 APS: Difference between revisions
No edit summary |
(Added refactoring points and code snippet.) |
||
Line 493: | Line 493: | ||
sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S") | sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S") | ||
Digest::SHA1.digest(sign) | Digest::SHA1.digest(sign) | ||
end | |||
===Review methods=== | |||
* The bookmark_reviews() method from AssignmentParticipant model have been moved to the '''BookmarkRatingQuestionnaire''' class. | |||
def get_assessments_for(participant) | |||
BookmarkRatingResponseMap.get_assessments_for(participant) # participant.bookmark_reviews() | |||
end | |||
* The teammmate_reviews() method from AssignmentParticipant model have been moved to the '''TeammateReviewQuestionnaire''' class. | |||
def get_assessments_for(participant) | |||
TeammateReviewResponseMap.get_assessments_for(participant) #participant.teammate_reviews() | |||
end | end | ||
Revision as of 02:29, 1 November 2015
E1562. Refactor AssignmentParticipant model<ref>https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus</ref>
This page provides a brief description of the Expertiza project. The project is aimed at refactoring the AssignmentParticipant model which is subclass of Participant model. This model is used to maintain the list of students/users participating in a given assignment.
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.
Project Description
Refactor AssignmentParticipant model which is a subclass of Participant model.
Tasks to be performed
- Refactor scores method to smaller methods.
- In set_handle method, there is no need for a separate ELSEIF statement. Club the ELSEIF statement into IF statement as an another OR condition.
- Remove methods like compute_quiz_scores(scores) , average_score_per_assignment(assignment_id) which are not being used.
- Method cycle_deviation_score(cycle) is also present in CollusionCycle, remove it from this class.
- Methods related to reviews like teammate_reviews, bookmark_reviews do not belong to AssignmentParticipant model. Move them to the appropriate class.
- Methods related to files and directories like files(directory) should not be present in AssignmentParticipant model, move them to FileHelper module.
Refactoring
Scores method
The method scores has been converted to smaller methods scores, assignment_questionnaires, merge_scores, calculate_scores.
Old code def scores(questions)
scores = {} scores[:participant] = self self.assignment.questionnaires.each do |questionnaire| round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round #create symbol for "varying rubrics" feature -Yang if(round!=nil) questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym else questionnaire_symbol = questionnaire.symbol end
scores[questionnaire_symbol] = {}
if round==nil scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self) else scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round) end scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol]) end
scores[:total_score] = self.assignment.compute_total_score(scores)
#merge scores[review#] (for each round) to score[review] -Yang if self.assignment.varying_rubrics_by_round? review_sym = "review".to_sym scores[review_sym] = Hash.new scores[review_sym][:assessments] = Array.new scores[review_sym][:scores] = Hash.new scores[review_sym][:scores][:max] = -999999999 scores[review_sym][:scores][:min] = 999999999 scores[review_sym][:scores][:avg] = 0 total_score = 0 for i in 1..self.assignment.get_review_rounds round_sym = ("review"+i.to_s).to_sym if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0 next end length_of_assessments=scores[round_sym][:assessments].length.to_f
scores[review_sym][:assessments]+=scores[round_sym][:assessments]
if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max]) scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max] end if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min]) scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min] end if(scores[round_sym][:scores][:avg]!=nil) total_score += scores[round_sym][:scores][:avg]*length_of_assessments end end
if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999 scores[review_sym][:scores][:max] = 0 scores[review_sym][:scores][:min] = 0 end
scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f end
# In the event that this is a microtask, we need to scale the score accordingly and record the total possible points # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else if assignment.is_microtask? topic = SignUpTopic.find_by_assignment_id(assignment.id) if !topic.nil? scores[:total_score] *= (topic.micropayment.to_f / 100.to_f) scores[:max_pts_available] = topic.micropayment end end
# for all quiz questionnaires (quizzes) taken by the participant quiz_responses = Array.new quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id) quiz_response_mappings.each do |qmapping| if (qmapping.response) quiz_responses << qmapping.response end end #scores[:quiz] = Hash.new #scores[:quiz][:assessments] = quiz_responses #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
scores[:total_score] = assignment.compute_total_score(scores) #scores[:total_score] += compute_quiz_scores(scores)
# move lots of calculation from view(_participant.html.erb) to model if self.grade scores[:total_score] = self.grade else total_score = scores[:total_score] if total_score > 100 total_score = 100 end scores[:total_score] = total_score scores end end
New code
- scores(questions) method
def scores(questions)
scores = {} scores[:participant] = self
assignment_questionnaires(questions, scores)
scores[:total_score] = self.assignment.compute_total_score(scores)
merge_scores(scores)
# In the event that this is a microtask, we need to scale the score accordingly and record the total possible points # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else if assignment.is_microtask? topic = SignUpTopic.find_by_assignment_id(assignment.id) if !topic.nil? scores[:total_score] *= (topic.micropayment.to_f / 100.to_f) scores[:max_pts_available] = topic.micropayment end end
# for all quiz questionnaires (quizzes) taken by the participant quiz_responses = Array.new quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id) quiz_response_mappings.each do |qmapping| if (qmapping.response) quiz_responses << qmapping.response end end #scores[:quiz] = Hash.new #scores[:quiz][:assessments] = quiz_responses #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
scores[:total_score] = assignment.compute_total_score(scores) #scores[:total_score] += compute_quiz_scores(scores)
calculate_scores(scores) end
- assignment_questionnaires(questions, scores) method
def assignment_questionnaires(questions, scores)
self.assignment.questionnaires.each do |questionnaire| round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round #create symbol for "varying rubrics" feature -Yang if(round!=nil) questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym else questionnaire_symbol = questionnaire.symbol end
scores[questionnaire_symbol] = {}
if round==nil scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self) else scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round) end scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol]) end end
- merge_scores(scores) method
def merge_scores(scores)
#merge scores[review#] (for each round) to score[review] -Yang if self.assignment.varying_rubrics_by_round? review_sym = "review".to_sym scores[review_sym] = Hash.new scores[review_sym][:assessments] = Array.new scores[review_sym][:scores] = Hash.new scores[review_sym][:scores][:max] = -999999999 scores[review_sym][:scores][:min] = 999999999 scores[review_sym][:scores][:avg] = 0 total_score = 0 for i in 1..self.assignment.get_review_rounds round_sym = ("review"+i.to_s).to_sym if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0 next end length_of_assessments=scores[round_sym][:assessments].length.to_f
scores[review_sym][:assessments]+=scores[round_sym][:assessments]
if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max]) scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max] end if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min]) scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min] end if(scores[round_sym][:scores][:avg]!=nil) total_score += scores[round_sym][:scores][:avg]*length_of_assessments end end
if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999 scores[review_sym][:scores][:max] = 0 scores[review_sym][:scores][:min] = 0 end
scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f end end
- calculate_scores(scores) method
def calculate_scores(scores)
# move lots of calculation from view(_participant.html.erb) to model if self.grade scores[:total_score] = self.grade else total_score = scores[:total_score] if total_score > 100 total_score = 100 end scores[:total_score] = total_score scores end end
set_handle method
The 'elseif' statement has been removed from set_handle method and clubbed in the if statement using an or condition.
Old code
def set_handle
if self.user.handle == nil or self.user.handle == "" self.handle = self.user.name elsif AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0 self.handle = self.user.name else self.handle = self.user.handle end self.save!
end
New code
def set_handle
if self.user.handle == nil or self.user.handle == "" or AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0 self.handle = self.user.name else self.handle = self.user.handle end self.save!
end
File and directory methods
The files and directory methods from AssignmentParticipant model have been moved to the FileHelper module.
The below line was added to the AssignmentParticipant model. include FileHelper The AssignmentParticipant class would like class AssignmentParticipant < Participant
require 'wiki_helper' include FileHelper
Methods moved to FileHelper:
- submitted_files
def submitted_files
files(self.path) if self.directory_num end
- files(directory)
def files(directory)
files_list = Dir[directory + "/*"] files = Array.new
files_list.each do |file| if File.directory?(file) dir_files = files(file) dir_files.each{|f| files << f} end files << file end files end
- submitted_files()
def submitted_files()
files = Array.new if(self.directory_num) files = files(self.path) end return files end
- dir_path
def dir_path
assignment.try :directory_path end
Remove unused methods
The following methods have been removed as they were not being used by the class:
def average_score_per_assignment(assignment_id)
def average_score_per_assignment(assignment_id) return 0 if self.response_maps.size == 0
sum_of_scores = 0
self.response_maps.metareview_response_maps.each do |metaresponse_map| if !metaresponse_map.response.empty? && response_map == assignment_id then sum_of_scores = sum_of_scores + response_map.response.last.average_score end end
(sum_of_scores / self.response_maps.size).to_i end
def quiz_taken_by?(contributor, reviewer)
def quiz_taken_by?(contributor, reviewer)
quiz_id = QuizQuestionnaire.find_by_instructor_id(contributor.id) return QuizResponseMap.where(['reviewee_id = ? AND reviewer_id = ? AND reviewed_object_id = ?', self.id, reviewer.id, quiz_id]).count > 0 end
def has_quiz?
def has_quiz?
return !QuizQuestionnaire.find_by_instructor_id(self.id).nil? end
def reviewees
def reviewees
reviewees = [] rmaps = ResponseMap.all(conditions: ["reviewer_id = #{self.id} && type = 'ReviewResponseMap'"]) rmaps.each { |rm| reviewees.concat(AssignmentTeam.find(rm.reviewee_id).participants) }
reviewees end
def two_node_cycles
def two_node_cycles
cycles = [] self.reviewers.each do |ap| if ap.reviewers.include?(self) self.reviews_by_reviewer(ap).nil? ? next : s01 = self.reviews_by_reviewer(ap).get_total_score ap.reviews_by_reviewer(self).nil? ? next : s10 = ap.reviews_by_reviewer(self).get_total_score cycles.push([[self, s01], [ap, s10]]) end end cycles end
def three_node_cycles
def three_node_cycles
cycles = [] self.reviewers.each do |ap1| ap1.reviewers.each do |ap2| if ap2.reviewers.include?(self) self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score ap2.reviews_by_reviewer(self).nil? ? next : s20 = ap2.reviews_by_reviewer(self).get_total_score cycles.push([[self, s01], [ap1, s12], [ap2, s20]]) end end end cycles end
def four_node_cycles
def four_node_cycles
cycles = [] self.reviewers.each do |ap1| ap1.reviewers.each do |ap2| ap2.reviewers.each do |ap3| if ap3.reviewers.include?(self) self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score ap2.reviews_by_reviewer(ap3).nil? ? next : s23 = ap2.reviews_by_reviewer(ap3).get_total_score ap3.reviews_by_reviewer(self).nil? ? next : s30 = ap3.reviews_by_reviewer(self).get_total_score cycles.push([[self, s01], [ap1, s12], [ap2, s23], [ap3, s30]]) end end end end cycles end
def cycle_similarity_score(cycle)
def cycle_similarity_score(cycle)
similarity_score = 0.0 count = 0.0
0 ... cycle.size-1.each do |pivot| pivot_score = cycle[pivot][1] similarity_score = similarity_score + (pivot_score - cycle[other][1]).abs count = count + 1.0 end
similarity_score = similarity_score / count unless count == 0.0 similarity_score end
def cycle_deviation_score(cycle)
def cycle_deviation_score(cycle)
deviation_score = 0.0 count = 0.0
0 ... cycle.size.each do |member| participant = AssignmentParticipant.find(cycle[member][0].id) total_score = participant.get_review_score deviation_score = deviation_score + (total_score - cycle[member][1]).abs count = count + 1.0 end
deviation_score = deviation_score / count unless count == 0.0 deviation_score end
def compute_quiz_scores(scores)
def compute_quiz_scores(scores)
total = 0 if scores[:quiz][:scores][:avg] return scores[:quiz][:scores][:avg] * 100 / 100.to_f else return 0 end return total end
def review_response_maps
def review_response_maps
participant = Participant.find(id) team_id = TeamsUser.team_id(participant.parent_id, participant.user_id) ReviewResponseMap.where(reviewee_id: team_id, reviewed_object_id: assignment.id) end
def members
def members
team.try :participants end
def get_hash(time_stamp)
def get_hash(time_stamp)
# first generate a hash from the assignment name itself hash_data = Digest::SHA1.digest(self.assignment.name.to_s)
# second generate a hash from the first hash plus the user name and time stamp sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S") Digest::SHA1.digest(sign) end
Review methods
- The bookmark_reviews() method from AssignmentParticipant model have been moved to the BookmarkRatingQuestionnaire class.
def get_assessments_for(participant)
BookmarkRatingResponseMap.get_assessments_for(participant) # participant.bookmark_reviews() end
- The teammmate_reviews() method from AssignmentParticipant model have been moved to the TeammateReviewQuestionnaire class.
def get_assessments_for(participant)
TeammateReviewResponseMap.get_assessments_for(participant) #participant.teammate_reviews() end
Testing
When the user runs the application, the behavior of the application remains the same.
The existing RSpec test cases remain successful. The same can be checked by running Rspec spec The participant is able to log in to the system, view the assignments, submit links and files in assignment submission, view the scores, review peers, etc. Below are some of the execution screenshots after the refactoring of the model.
File:Login to expertiza from here.png File:Expertiza test1.png File:Expertiza test2.png File:Expertiza test3.png File:Expertiza test4.png
Resources
- Github repository<ref>https://github.com/expertiza/expertiza</ref>
References
<references/>