CSC/ECE 517 Fall 2013/oss E804 spb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
Line 58: Line 58:
: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.
: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.


== Comparison of performance with refactored code ==
== Performance Comparison with refactored code ==
:Existing Code:
[[File:before.png]]


[[File:before.png]]
:Refactored Code:
[[File:After.jpg]]


== Future Work ==
== Future Work ==

Revision as of 00:07, 31 October 2013

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]
  weighted_score = questionnaireData[0].weighted_score.to_f
  sum_of_weights = questionnaireData[0].sum_of_weights.to_f
  max_question_score = questionnaireData[0].q1_max_question_score.to_f

Separated functionality

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.

Setup Issues