CSC/ECE 517 Spring 2020 - E2003. Refactor and improve assessment360 controller
About Expertiza
Expertiza is an open-source project based on Ruby on Rails framework. The software allows the instructor and students with the following privileges,
Instructors
- Create new assignments
- Customize new or existing assignments
- Create a list of topics that students can sign up for
- Give grades/reviews for the students' submissions
Students
- Form teams with other students to work on various projects and assignments
- Peer review other students' submissions
Note: Expertiza supports submissions across various document types, including URL and wiki pages
Project Description
About assessment_360 Controller
The assessment_360 controller is designed to gather data from all stakeholders about the performance of a student in a course. Thus, its purpose is to provide course-level statistics. At present, the controller provides only two top-level functions where each of them gives a list of students in the class.
- all_students_all_reviews contains teammate and meta-review scores received by students for all assignments in the course
- course_student_grade_summary gives a listing of students and what topic they worked on and what grade they received on each assignment during the semester
Problem Statement
The assessment360 controller had three major issues. This project is aimed to address some of them, including:
Issue 1: Methods all_students_all_reviews and course_student_grade_summary were too long. Common initialization and looping had to factored out by moving them into smaller methods with mnemonic names
Issue 2: Method populate_hash_for_all_students_all_reviews is not easy to understand. It uses a special data structure to do a very simple task. That had to be refactored
Issue 3: Method course_student_grade_summary stopped working and has to be fixed
Motivation
This project aims at encouraging students to collaborate and contribute to open-source project in turn helping them understand what goes on in a full-fledged software deployment process. It also paves a way to understand how one can use Rails, RSpec to develop a system that does justice to the DRY code practicing principles.
Approach
Problem 1: Refactoring all_students_all_reviews and course_student_grade_summary
(a) The common elements in these files i.e., checking if the course participant list is empty or not is moved to a different function called inspect_course_participants
(b) Adding on to this, all_student_all_review was broken down to two more functions, avg_review_calc_per_student and overall_review_count
(c) The method course_student_grade_summary is broken down to one more function assignment_grade_summary
Problem 2: Refactoring populate_hash_for_all_students_all_reviews
The approach in this function is pretty straight forward when we try to break it down. It calculates,
(a) The average score for each assignment for each course
(b) The overall review grade for each assignment for all the students who have taken the course
Passing the average score calculated from (a) checks if the student has teammates which could result in not having an average teammate_review_score. If we calculate (b) in our main function (this can be done) we would be repeating the checks that we are already doing for (a). Hence, this function is not changed
Problem 3: Fix course_student_grade_summary
The current implementation ran into an issue when it was comparing the Key-Value pair passed in peer_review_score. It was hard to check if each of the keys were present or not. The proposed solution makes use of a basic hash value presence check that retrieves a value from the array/hash, given a set of keys. If no value is found, it'll return nil.
Proposed Implementation
Common refactoring of all_students_all_reviews and course_student_grade_summary method
The functionality that the methods shared was that, both of them checked if there were course participants who had taken a particular course. This occupied 4 lines of code in both the functions. So, this is defined in a separate function called insure_existence_of and called from the two functions.
def insure_existence_of(course_participants)
if course_participants.empty?
flash[:error] = "There is no course participant in course #{course.name}"
redirect_to(:back)
end
end
Refactor all_students_all_reviews method
The all_students_all_reviews method is one of the biggest methods in assessment360_controller.rb file. It finds the list of all students and assignments pertaining to the course. This data is then used to compute the metareview and teammate review scores for each student.
The code before refactoring had 64 lines of code as shown below,
def all_students_all_reviews
course = Course.find(params[:course_id])
@assignments = course.assignments.reject(&:is_calibrated).reject {|a| a.participants.empty? }
@course_participants = course.get_participants
if @course_participants.empty?
flash[:error] = "There is no course participant in course #{course.name}"
redirect_to(:back)
end
# hashes for view
@meta_review = {}
@teammate_review = {}
@teamed_count = {}
# for course
# eg. @overall_teammate_review_grades = {assgt_id1: 100, assgt_id2: 178, ...}
# @overall_teammate_review_count = {assgt_id1: 1, assgt_id2: 2, ...}
%w[teammate meta].each do |type|
instance_variable_set("@overall_#{type}_review_grades", {})
instance_variable_set("@overall_#{type}_review_count", {})
end
@course_participants.each do |cp|
# for each assignment
# [aggregrate_review_grades_per_stu, review_count_per_stu] --> [0, 0]
%w[teammate meta].each {|type| instance_variable_set("@#{type}_review_info_per_stu", [0, 0]) }
students_teamed = StudentTask.teamed_students(cp.user)
@teamed_count[cp.id] = students_teamed[course.id].try(:size).to_i
@assignments.each do |assignment|
@meta_review[cp.id] = {} unless @meta_review.key?(cp.id)
@teammate_review[cp.id] = {} unless @teammate_review.key?(cp.id)
assignment_participant = assignment.participants.find_by(user_id: cp.user_id)
next if assignment_participant.nil?
teammate_reviews = assignment_participant.teammate_reviews
meta_reviews = assignment_participant.metareviews
populate_hash_for_all_students_all_reviews(assignment,
cp,
teammate_reviews,
@teammate_review,
@overall_teammate_review_grades,
@overall_teammate_review_count,
@teammate_review_info_per_stu)
populate_hash_for_all_students_all_reviews(assignment,
cp,
meta_reviews,
@meta_review,
@overall_meta_review_grades,
@overall_meta_review_count,
@meta_review_info_per_stu)
end
# calculate average grade for each student on all assignments in this course
if @teammate_review_info_per_stu[1] > 0
temp_avg_grade = @teammate_review_info_per_stu[0] * 1.0 / @teammate_review_info_per_stu[1]
@teammate_review[cp.id][:avg_grade_for_assgt] = temp_avg_grade.round.to_s + '%'
end
if @meta_review_info_per_stu[1] > 0
temp_avg_grade = @meta_review_info_per_stu[0] * 1.0 / @meta_review_info_per_stu[1]
@meta_review[cp.id][:avg_grade_for_assgt] = temp_avg_grade.round.to_s + '%'
end
end
# avoid divide by zero error
@assignments.each do |assignment|
temp_count = @overall_teammate_review_count[assignment.id]
@overall_teammate_review_count[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
temp_count = @overall_meta_review_count[assignment.id]
@overall_meta_review_count[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
end
end
To calculate the average grade for each student on all assignments in this course that a student has taken in this course is refactoring in the base code by adding the avg_review_calc_per_student method.
def avg_review_calc_per_student(cp, review_info_per_stu, review)
# check to see if the student has been given a review
if review_info_per_stu[1] > 0
temp_avg_grade = review_info_per_stu[0] * 1.0 / review_info_per_stu[1]
review[cp.id][:avg_grade_for_assgt] = temp_avg_grade.round.to_s + '%'
end
end
To avoid a divide by zero error that is thrown when we try to calculate the average grade when a student has no teammate is handled by the overall_review_count method.
def overall_review_count(assignments, overall_teammate_review_count, overall_meta_review_count)
assignments.each do |assignment|
temp_count = overall_teammate_review_count[assignment.id]
overall_review_count_hash[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
temp_count = overall_meta_review_count[assignment.id]
overall_meta_review_count[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
end
end
After refactoring
def all_students_all_reviews
course = Course.find(params[:course_id])
@assignments = course.assignments.reject(&:is_calibrated).reject {|a| a.participants.empty? }
@course_participants = course.get_participants
insure_existence_of(@course_participants)
# hashes for view
@meta_review = {}
@teammate_review = {}
@teamed_count = {}
# for course
# eg. @overall_teammate_review_grades = {assgt_id1: 100, assgt_id2: 178, ...}
# @overall_teammate_review_count = {assgt_id1: 1, assgt_id2: 2, ...}
%w[teammate meta].each do |type|
instance_variable_set("@overall_#{type}_review_grades", {})
instance_variable_set("@overall_#{type}_review_count", {})
end
@course_participants.each do |cp|
# for each assignment
# [aggregrate_review_grades_per_stu, review_count_per_stu] --> [0, 0]
%w[teammate meta].each {|type| instance_variable_set("@#{type}_review_info_per_stu", [0, 0]) }
students_teamed = StudentTask.teamed_students(cp.user)
@teamed_count[cp.id] = students_teamed[course.id].try(:size).to_i
@assignments.each do |assignment|
@meta_review[cp.id] = {} unless @meta_review.key?(cp.id)
@teammate_review[cp.id] = {} unless @teammate_review.key?(cp.id)
assignment_participant = assignment.participants.find_by(user_id: cp.user_id)
next if assignment_participant.nil?
teammate_reviews = assignment_participant.teammate_reviews
meta_reviews = assignment_participant.metareviews
calc_overall_review_info(assignment,
cp,
teammate_reviews,
@teammate_review,
@overall_teammate_review_grades,
@overall_teammate_review_count,
@teammate_review_info_per_stu)
calc_overall_review_info(assignment,
cp,
meta_reviews,
@meta_review,
@overall_meta_review_grades,
@overall_meta_review_count,
@meta_review_info_per_stu)
end
# calculate average grade for each student on all assignments in this course
avg_review_calc_per_student(cp, @teammate_review_info_per_stu, @teammate_review)
avg_review_calc_per_student(cp, @meta_review_info_per_stu, @meta_review)
end
# avoid divide by zero error
overall_review_count(@assignments, @overall_teammate_review_count, @overall_meta_review_count)
end
Refactor course_student_grade_summary method
This method finds the list of all students and assignments pertaining to the course. The data obtained is then used to compute the instructor assigned grade and peer review scores. It was the secong biggest method in the assessment360_controller.rb controller file. The code before refactoring is shown below,
def course_student_grade_summary
@topics = {}
@assignment_grades = {}
@peer_review_scores = {}
@final_grades = {}
course = Course.find(params[:course_id])
@assignments = course.assignments.reject(&:is_calibrated).reject {|a| a.participants.empty? }
@course_participants = course.get_participants
if @course_participants.empty?
flash[:error] = "There is no course participant in course #{course.name}"
redirect_to(:back)
end
@course_participants.each do |cp|
@topics[cp.id] = {}
@assignment_grades[cp.id] = {}
@peer_review_scores[cp.id] = {}
@final_grades[cp.id] = 0
@assignments.each do |assignment|
user_id = cp.user_id
assignment_id = assignment.id
assignment_participant = assignment.participants.find_by(user_id: user_id)
next if assignment.participants.find_by(user_id: user_id).nil?
# A topic exists if a team signed up for a topic, which can be found via the user and the assignment
topic_id = SignedUpTeam.topic_id(assignment_id, user_id)
@topics[cp.id][assignment_id] = SignUpTopic.find_by(id: topic_id)
# Instructor grade is stored in the team model, which is found by finding the user's team for the assignment
team_id = TeamsUser.team_id(assignment_id, user_id)
next if team_id.nil?
team = Team.find(team_id)
peer_review_score = find_peer_review_score(user_id, assignment_id)
# Set the assignment grade, peer review score, and sum for the final student summary
@assignment_grades[cp.id][assignment_id] = team[:grade_for_submission]
unless @assignment_grades[cp.id][assignment_id].nil?
@final_grades[cp.id] += @assignment_grades[cp.id][assignment_id]
end
unless (peer_review_score.nil? || peer_review_score[:review][:scores][:avg].nil?)
@peer_review_scores[cp.id][assignment_id] = peer_review_score[:review][:scores][:avg].round(2)
end
end
end
end
Team Information
Sri Harsha Varma Uppalapati - LinkedIn
Mentor: Dr. Edward F Gehringer - Website