Computing overall averages in my web application

Question

I'm a senior Comp. Sci. major working on a senior design project for our faculty. The name of this project is "Gradebook", and it is responsible for allowing instructors to record grades for students and for students to check their grades in a class. This project is written in Ruby on Rails, and this feature set is integrated into our current CS website.

One requirement for our project is to constantly keep the course average and each of the student's averages updated, so I've designed a CourseInfo class and a StudentInfo class to help with this process.

The CourseInfo class accepts a Gradebook (an ActiveRecord object) as a parameter and calculates the course average. It creates an Associative Array of StudentInfo objects, with each StudentInfo object containing the student's overall average in the class. It also eagerly loads all of Gradebook's ActiveRecord associations in order to avoid N + 1 queries.

Every time a student's grade is updated, the new student average and course average automatically updates in the UI through AJAX. The problem is, the CourseInfo object has to keep being recreated with each request sent to the server, which is a bottleneck. My plan is to serialize the CourseInfo object and store it in the Gradebook table.

However I also want to make sure the way I'm calculating the overall averages is efficient too. What do you think of my approach to doing this? Do you spot any potential bottlenecks or inefficiencies in my code?

To explain simply how our database tables are setup:

1. A Gradebook has many Categories (and belongs to a Gradebook)

2. A Category has many Assignments (and belongs to a Category)

3. An Assignment has many Grades (and belongs to an Assignment and Gradebook)

And to explain how a Gradebook is setup:

1. A Gradebook can be setup in a point-based system or percentage-based system. In a point-based system, all categories are weighted by points, in a percentage-based system, all categories are weighted by percentages.

2. A Category can be created in a point-based system or a percentage-based system. In a point-based system, all assignments are weighted by points, in a percentage-based system, all assignments are weighted by percentages.

course_info.rb

module Api
# Organizes Gradebook data to retrieve useful information
 class CourseInfo
 # Initializes the CourseInfo object by eagerly loading the all Gradebook data and initializing student data.
 #
 # ==== Attributes
 #
 # +gradebook+ - A Gradebook ActiveRecord object
 def initialize ( gradebook )
 @percentage_sum = 0
 @students_with_grades = 0
 refresh_categories(gradebook)
 init_student_hash(gradebook)
 end
 # Returns the overall average in the class
 def get_overall_average
 @percentage_sum / @students_with_grades.to_f unless @students_with_grades == 0
 end
 # Returns all eagerly loaded data
 def get_all_categories
 @all_categories
 end
 # Returns a StudentInfo object when given a User object representing a student in the class. This StudentInfo object can
 # be used to retrieve the student's average in the class.
 #
 # ==== Attributes
 #
 # +student+ - The User ActiveRecord object that represents a student in the class.
 def get_student_info(student)
 @student_hash[student.id]
 end
 private
 # Eagerly loads all categories, assignments, and grades into the CourseInfo object
 #
 # ==== Attributes
 #
 # +gradebook+ - The associated Gradebook ActiveRecord object
 def refresh_categories(gradebook)
 @all_categories = gradebook.categories.all(include: [{assignments: :grades}])
 end
 # Initializes an Associate Array of StudentInfo objects
 #
 # ==== Attributes
 #
 # +gradebook+ - The associated Gradebook ActiveRecord object
 def init_student_hash(gradebook)
 # Initialize the Hash table
 @student_hash = Hash.new
 # For all students in the Gradebook
 gradebook.fetch_all_students.each do |student|
 # Create a new StudentInfo object using the student's ID as a key
 @student_hash[student.id] = StudentInfo.new(gradebook.grading_system, @all_categories, student)
 # Check if the student has an average in the class, if so, we
 # include this in the calculation of the overall average of the class
 my_student_info = @student_hash[student.id]
 if !(my_student_info.get_overall_average.nil?)
 @percentage_sum += my_student_info.get_overall_average
 @students_with_grades += 1
 end
 end
 end
 end
end

student_info.rb

module Api
 # A StudentInfo object stores the student's overall average in the class, and will constantly update the student's
 # overall average automatically.
 class StudentInfo
 # Initializes the StudentInfo object by averaging all of the student's assignments in the class
 #
 # ==== Attributes
 #
 # +grading_system+ - The class's grading system (percentages or points)
 # +all_categories+ - All categories in the class (eagerly loaded from Gradebook ActiveRecord object)
 # +student+ - The User ActiveRecord object represeting the student in the class
 def initialize ( grading_system, all_categories, student ) 
 @grading_system = grading_system
 init_student_overall_average(all_categories, student)
 end
 # Returns the student's overall average in the class unless the Gradebook is in a percentage system.
 def get_overall_average
 @overall_average
 end
 # Returns the points earned in the class if the class is in a point-based system
 def get_points_earned
 @points_earned unless @grading_system == "percentages"
 end
 # Returns the max points in the class if the class is in a point-based system
 def get_max_points
 @points_max unless @grading_system == "percentages"
 end
 private
 # Averages all the student's grades in the class
 #
 # ==== Attributes
 #
 # +all_categories+ - All categories in the class (eagerly loaded from Gradebook ActiveRecord object)
 # +student+ - The User ActiveRecord object represeting the student in the class
 def init_student_overall_average(all_categories, student)
 if gradebook.grading_system == "percentages"
 # In a percentage-based class, compute the student's overall average in the class
 computeOverallAverage(all_categories, student) 
 elsif gradebook.grading_system == "points"
 # In a point-based system, compute the student's overall earned and max points in the class
 computeStudentPoints(all_categories, student)
 end
 end
 private
 # Calculate the student's overall average in a percentage-based system
 # ==== Attributes
 #
 # +all_categories+ - All categories in the class (eagerly loaded from Gradebook ActiveRecord object)
 # +student+ - The User ActiveRecord object represeting the student in the class
 def computeOverallAverage(all_categories, student)
 # A flag that sets if the student has grades in the class.
 has_no_grades = true
 # The sum of all the grade's percentage-earned values - used for calculating average.
 percentage_sum = 0.0
 # The sum of all the grade's percentage-max values - used for calculating average.
 total_weight = 0.0
 all_categories.each do |category|
 category.assignments.each do |assignment|
 # Fetch the assignment grade
 my_grade = assignment.get_student_grade(student.id)
 # If this grade has been set
 if !(my_grade.points_earned.nil?) || !(my_grade.percentage.nil?)
 # Student has grades in the class 
 has_no_grades = false
 # Get the assignment's total weight 
 assignment_weight = assignment.get_overall_weight
 # If the Category's Grading System is point-based
 if category.grading_system == "points"
 # Calculate the percentage, multiplied by the assignment's weight in the class 
 percentage = (my_grade.points_earned / assignment.point_total) * assignment_weight
 # If the Category's Grading System is percentage-based
 elsif category.grading_system == "percentages"
 # Multiply percentage-earned by the assignment's weight in the class
 percentage = my_grade.percentage * assignment_weight
 end
 # Sum up the percentages and total assignment weights
 percentage_sum += percentage
 total_weight += assignment_weight
 end
 end
 end
 # The overall average is the sum of the percentages divided by the grand total of all
 # assignment weights
 @overall_average = percentage_sum / total_weight unless has_no_grades
 end
 # Calculate the student's total points earned in a point-based system. 
 # ==== Attributes
 #
 # +all_categories+ - All categories in the class (eagerly loaded from Gradebook ActiveRecord object)
 # +student+ - The User ActiveRecord object represeting the student in the class
 def computeStudentPoints(all_categories, student)
 # Initialize points-earned and max points
 points_earned = 0.0
 points_max = 0.0
 all_categories.each do |category|
 category.assignments.each do |assignment|
 # Fetch the assignment grade
 my_grade = assignment.get_student_grade(student.id)
 # If this grade has been set
 if !(my_grade.points_earned.nil?) || !(my_grade.percentage.nil?)
 # If the Grading System of the Category is points
 if category.grading_system == "points"
 # Add up the points earned in the grade and the max points in the assignment
 points_earned += my_grade.points_earned
 points_max += assignment.point_total
 # If the grading system is percentages
 elsif category.grading_system == "percentages"
 # Add up the points earned and max points in the grade (percentage multiplied 
 # by the assignment's total weight)
 points_earned += my_grade.percentage * (assignment.weight * category.point_total)
 points_max += assignment.weight * category.point_total
 end
 end
 end
 end
 # Set total points earned and total max points, and compute stuent's overall average based
 # on the information. 
 if points_max != 0
 @points_earned = points_earned
 @points_max = points_max
 @overall_average = @points_earned / @points_max
 end
 end
 end
end

assignment.rb

class Assignment < ActiveRecord::Base
 include ActionView::Helpers
 after_initialize :set_unique_id
 after_create :init_assignment_grades
 belongs_to :category
 has_many :grades, dependent: :destroy
 has_many :topics, dependent: :destroy
 def get_gradebook
 return Category.find(self.category_id).gradebook
 end
 def create_assignment_grades_for_user(gradebook, user)
 if user.student? || user.user?
 new_grade = Grade.create(student_name: user.email_short, dropped: false)
 self.grades << new_grade
 user.grades << new_grade
 gradebook.grades << new_grade
 end
 end
 def get_overall_weight
 # Get the category this assignment belongs to
 category = self.category
 # Get the Gradebook
 gradebook = get_gradebook
 # Is this category setup percentage-based?
 if category.grading_system == "percentages"
 # Then simply fetch the weight from the table
 assignment_weight = self.weight
 # Is this category point-based? 
 elsif category.grading_system == "points"
 # Then divide the point total of the assignment by the category's point total
 assignment_weight = self.point_total / category.point_total
 end
 # Is the Gradebook setup percentage-based?
 if gradebook.grading_system == "percentages"
 # Then simply fetch the category's weight from the table
 category_weight = category.weight
 # Is the Gradebook setup point-based? 
 elsif gradebook.grading_system == "points"
 # Then divide the category's point total by the Gradebook's point total
 category_weight = category.point_total / gradebook.point_total
 end
 # The assignment's total weight is it's weight multiplied by its category's weight 
 return assignment_weight * category_weight
 end
 def get_student_grade(student_id)
 myGrade = self.grades.index_by(&:user_id)[student_id]
 return myGrade
 end
 def get_percentage_average
 return self.grades.average(:percentage)
 end
 def get_points_average
 return self.grades.average(:points_earned)
 end
 def get_percentage_min
 return self.grades.minimum(:percentage)
 end
 def get_points_min
 return self.grades.minimum(:points_earned)
 end
 def get_percentage_max
 return self.grades.maximum(:percentage)
 end
 def get_points_max
 return self.grades.maximum(:points_earned)
 end
 private
 def set_unique_id
 self.key ||= SecureRandom.hex if self.new_record?
 end
 def init_assignment_grades
 my_gradebook = get_gradebook
 my_gradebook.users.each do |user|
 create_assignment_grades_for_user(my_gradebook, user)
 end
 end
end

• tsleyson: I'm not too experienced with web programming, but your problem with recreating the CourseInfo object might be solved by a view model. Try checking out the MVVM pattern, and JavaScript frameworks like Knockout.js.

Answer 1

Lots of excellent stuff here! Some points I would mention:

class CourseInfo
 def get_all_categories
 @all_categories
 end
end

Use an attr_reader instead of this trivial accessor; this code becomes:

class CourseInfo
 attr_reader :all_categories
end

If you didn't know, attr_reader creates a method called <attribute> (in this case, all_categories) that simply returns the instance variable (@all_categories, exactly what you already have). Remember to change the places you call it from get_all_categories to all_categories.

This use of attr_reader wont' suffice for some of your other getters, as they have a bit of logic behind them that you need to state.

This leads into the next point - rename all your get_some_attribute methods to some_attribute. This will keep in line with the attr_reader standard.

---

if !(my_student_info.get_overall_average.nil?)

Ordinarily, you would just use unless instead of if !(condition), but in this case you can exploit the fact the ruby treats EVERYTHING that isn't nil or false as true; change this line to:

if my_student_info.overall_average

(Note that I dropped the .nil?, the parentheses, and the get_... as per my previous point). You typically don't use parentheses on conditionals unless it would be prohibitively confusing without; see the style guide

Make a similar fix for this and other lines:

 if !(my_grade.points_earned.nil?) || !(my_grade.percentage.nil?)

---

@student_hash = Hash.new

Don't do that (see style guide); do this:

@student_hash = {}

---

In class StudentInfo, you say that all_categories and student are attributes but you are not treating them like attributes. You pass them to the function init_student_overall_average when that function could just access them as instance variables using @all_categories and @student syntax.

This will first require you to set those variables, however. Either pass them in to the initializer and set them explicitly, or you could use attr_writer in much the same way as attr_reader above to create the all_categories= and student= methods. Then you can set the variables whenever you want by calling, for example,

this_student_info.student = Student.new("Steve") # Or however students are made

---

You have way too many comments, delete pretty much all of them except for the ones that are REQUIRED to understand what the code does;

# Get the Gradebook
gradebook = get_gradebook

REALLY not needed. Now get rid of the rest of the comments too, and reword your code so that you can understand what it does without needing the comment.

---

In class Assignment in your vast amount of getters, you don't need to be specifying the self keyword or the return keyword. Methods are automatically invoked on self first if they exist on self, and everything in ruby automatically returns the final line it computes before exiting. You use the return keyword in a few other places as well and for your cases it's not needed. You DO use it when you want to return early from a method, but you're not doing that here.

(Edit: I'm not entirely sure about not needing to use the self keyword, because your grades are not instance variables - see below`)

---

Finally, in class Assignment your grades and other things should arguably be instance variables (@grades) but I'm not entirely familiar with how has_many sets things up so this might be more trouble than it's worth.

• Devon Parsons: I might come back and add some more stuff to this later, too, but that should be enough to get started with :P
• Sam Parsons: Thanks :) Yeah, I come from a Java and C++ background so writing "return" and specifying getters and setters is something I am used to. I will work on applying your suggestions. Thanks. If I don't get any more answers soon I will accept yours.