CSC/ECE 517 Fall 2013/oss E804 spb

Expertiza Code Refactoring

Introduction

A way to query db models to return scores, without UI changes

Project Description

Classes:

models/score.rb (159 lines)

grades_controller.rb (241 lines)

What needs to be done:

This code is very slow, due to many factors. Two of the most prominent are the fact that separate db queries are used for each rubric that has been filled out by anyone associated with the assignment; these queries are made sequentially while the HTML page is being written; and the fact that HTML for the whole page is generated, largely by controller methods, before anything is displayed.

Need to refactor the score.rb model which contains complex methods.

Design and Code Changes

The model scores.rb has only two methods that compute scores for the participants based on assignments and courses. But these methods have used hashes extensively to handle data. Also, the method get_total_score() makes many sequential queries to the database which increases the loading time for the page. These functions have logic which can be separated into two methods to make it more modular.

Created Database views

Initially the method get_total_scores() was making multiple sequential queries to the database to calculate weighted_scores,sum_of_wieghts and max_question_score. Also, these queries were made to 4 different tables : questions,questionnaire,scores and question_types. This increases the time the page takes to load.

To reduce the loading time , we merged the multiple queries into a single query by implementing database views. We created a migration "CreateMyViews" that creates a view which has information from all the above 4 tables in one place. So now a single query on that view returns all the three values - weighted_scores,sum_of_wieghts and max_question_score.

Before implementing views:

@questionnaire = Questionnaire.find(@questions[0].questionnaire_id)
@questions.each {
         |question|
       item = Score.find_by_response_id_and_question_id(@response.id, question.id)
       if item != nil
         weighted_score += item.score * question.weight
       end
       sum_of_weights += question.weight
     }

After implementing views:

The each loop making multiple queries to the database is eliminated and a single query does the work instead.

questionnaireData = MyView.find_by_sql ["SELECT q1_max_question_score ,SUM(question_weight) as sum_of_weights,
SUM(question_weight * s_score) as weighted_score 
FROM my_views WHERE q1_id = ? AND s_response_id = ?",questions[0].questionnaire_id,response .id]

A separate ruby class PartcipantScores.rb and AssignmentScores.rb was added which would include all the complex logic for finding the scores for a participant and assignment respectively. The scores are obtained in a single query. The use of database views makes the data retrieval process faster.

Separated functionality

The Single Responsibility Principle has been followed. The get_total_scores function computes total scores and also has the code to check if a response is valid or invalid. This code is independent of computation of scores and can be separated into a separate function. We have created a function 'submission_valid?' to check if a given response is valid or not.

Reduced the use of hash

The get_total_scores function accepts a hash as a parameter, which can be avoided. We have refactored the method to accept three arguments instead of a hash. We had to change all the dependent classes that call this method.

The compute_scores method returns a hash that stores the minimum, maximum and average score. We created a class ParticipantScore.rb that has attributes min, max and avg which store the minimum, maximum and average score of a particpant. This object could be used to maintain state as against an hash. Now the compute_score method returns an object of class ParticipantScore instead of an hash.

Performance Comparison with refactored code

Existing Code:

Refactored Code:

Future Work

Several model classes namely; assignment.rb, participant.rb, score.rb include extensive use of hashes and with high depths: a hash within a hash within a hash. These classes could be modified to reduce the use of hash and use an object instead. Also, database views can be used in place of code that makes multiple sequential queries to reduce the loading time of pages. PartcipantScores.rb could be developed further and used in place of Score the Score model wherein all the complex code has been written. Similar thing can be done for finding assignment scores. Some complex logic with regards to "Custom" questionnaire, which has not been implemented in the current project can be developed.