CSC/ECE 517 Fall 2014/OSS E1467 rsv
Introduction
Project name: E1467: Expertiza - Refactoring LeaderBoard model<ref>E1467: Expertiza - Refactoring LeaderBoard model</ref>
Our project is to refactor the code in the "Leaderboard" functionality of the web application Expertiza<ref>Expertiza</ref>. The Expertiza project is a system to create reusable learning objects through peer review. The leaderboard functionality is to show top 3 individual scorers in each questionnaire type (ReviewQuestionnaire, AuthorFeedbackQuestionnaire, MetareviewQuestionnaire and TeammateReviewQuestionnaire), in each course of the logged-in user. The leaderboard also shows the current standing of the logged in user under personal achievements.
| Expertiza instance links | ||
| Refactored Instance | http://152.1.13.181:3000 | username= "user480" password= "password" |
| Original Instance | http://152.1.13.181:3001 | |
Project Description
Classes involved: leaderboard.rb and associated other model and controllers classes.
1. Come up with an efficient way to search for participants based on assignment ids.
2. Refactor score hash for personal achievements. (method: extractPersonalAchievements ). Seperate out method which will rank individual personal achievements.
3. Refactor addEntryToCSHash according to your new metric method.
4. Seperate out computation of CS entries(metric) and refactor it to be more modular and elegant.
5. Refactor getParticipantEntriesInAssignmentList method. Its very complex (Complexity=142).
6. Refactor addEntryToCSHash according to your new metric method.
Existing Functionality
Leaderboard.rb class gets all the assignments within all the courses taken by currently logged in user. It also fetches any independent assignments (not associated with any course), that the user has taken. Then, it fetches all the participants who have participated in the computed list of assignments and aggregates their scores based on the questionnaire type. Several questionnaire type is associated with a single assignment. e.g. Direwolf application has - Review Questionnaire, Author Feedback Questionnaire and Teammate Review Questionnaire.
Leaderboard model has following 3 important methods:
| Sr. No. | Method Name | Comment |
|---|---|---|
| 1 | getParticipantEntriesInAssignmentList(assignmentList)
|
This method is responsible for calculating the leaderboard of all the participants associated with assignments in given assignment list. |
| 2 | extractPersonalAchievements(csHash, courseIdList, userId)
|
This method is responsible for calculating personal aggregated score. It also calculates the ranking of currently logged in user against total users associated with the same set of courses as that of current user. |
| 3 | addEntryToCSHash(qtypeHash, qtype, userid, csEntry, courseid)
|
This method is called internally from Leaderboard.getParticipantEntriesInAssignmentList . This method aggregates score of a user grouped by course id, further grouped by questionnaire type.
|
Changes in Model
Changes made in the methods of model leaderboard.rb
| Sr. No. | Method Name | Changes Made | Reason For Change |
|---|---|---|---|
| 1 | getIndependantAssignments
|
Separate db call for each user assignment. | Single db call with an array of assignment ids. |
| 2 | getParticipantEntriesInCourses
|
Confusing code. Appends each entry to the end of list | Used concat to clarify that we are adding the independent and other assignments in courses. |
| 3 | sortHash
|
in place changes. | Returns a new deep copy of the updated hash. |
| 4 | extractPersonalAchievements
|
Confusing code with extra data base calls. | This method is refactored to use only 1 database call unlike several within the loop in its prior implementation. We have also removed redundant code computation and made the logic easier to understand. The overall complexity of this method is reduced from 72 to 35. |
| 5 | addEntryToCSHash
|
Confusing name and hard to follow logic. | The new method name is addScoreToResultantHash . As mentioned earlier, this is called internally by getParticipantEntriesInAssignmentList . The complexity of this method is reduced from 67 to 39 .
|
| 6 | getParticipantEntriesInAssignmentList
|
This method was the most complex method in the class. | The new refactored method name is getParticipantsScore . The method was refactored on various aspects like reducing database calls drastically, removing redundant code computation, redundant data storage in complex group of hashes and series of conditional statements. We would like to mention that previously, the method had 10 database calls within loop and 1 outside loop. After refactoring, the new method has just 1 database call within loop and 3 outside loops. Also, the overall code complexity is reduced from 142 to 64.
|
Changes in Helper
Changes made in methods of Helper leaderboard_helper.rb
| Sr. No. | Method Name | Changes Made | Reason For Change |
|---|---|---|---|
| 1 | userIsInstructor
|
Rewrote to reduce the number of database calls. | This method was refactored to reduce multiple database calls. 4 database calls was replaced by single call. |
| 2 | studentInWhichCourses
|
Rewrote to reduce the number of database calls. | single call. |
| 3 | getTop3Leaderboards
|
Dead code | Removed. |
There are several other changes in the views and controller which deal with renaming of the methods
and variables, adding proper comments etc and minor code refactoring. Please refer to our forked
github repository to view those changes.
Testing
On VCL session there are 2 instances running. On port 3001 the original instance is setup and on port 3000 the refactored instance is setup.
In below images we have shown that the output after refactoring is same. We changed the order in which the output is displayed to make it more coherent.
Note for testing : It is recommended that in order to test the leaderboard functionality by any user who has participated in several assignments, some of them which is associated with any course and there are other existing users who have participated in the same assignment.
Expertiza Test instances links
| View of Refactored Leaderboard | View of Original Leaderboard |
|---|---|
 |
 |
 |
|
Optimization
The refactoring process included reducing the database calls, loops and redundant storage and computation in all classes associated with Leaderboard functionality. While refactoring, the team has ensured to improve the readability of the code too by renaming ambiguous method and variable names and adding relevant comments to explain the objective of a code construct.
Code Optimization
In the project description code complexity has been highlighted for several methods. Code Climate<ref>Code Climate</ref> has been used to measure the code complexity of the current repository of Expertiza. After refactoring, we used the same tool i.e. Code Climate to measure the code complexity. Following is the snapshot of code complexity of Leaderboard.rb from Code Climate.
| Code complexity of original leaderboard implementation | Code complexity of refactored leaderboard implementation |
|---|---|
 |
 |
 |
 |
The report shows that all the 3 methods to be refactored have improved code complexity by over 50%. Following is the overall improvement report by Code Climate.
Database Optimization
We computed the number of data base "SELECT" call generated the database driver by filtering the messages from the rails server. In the original code there were 625 database select access generated for the leaderboard view. In the new code the number of select calls dropped to 111. Shown below is the number of select calls generated per table.
| Table Name | Original Version | Refactored Version | Comment |
|---|---|---|---|
| roles | 52 | 13 | Optimized the SQL query in leaderboard_helper.rb to replace multiple database calls. |
| users | 25 | 25 | No Change |
| participants | 111 | 3 | All the participants associated with computed assignment list are calculated in a single database call and cached in memory rather than calling repetitively within loop. |
| assignments | 16 | 5 | Database calls to Assignments table was reduced as result of removing it from multiple loops. Supporting data structure and caching enabled to achieve this reduction. |
| assignment_questionnaires | 11 | 0 | Upon careful code investigation, we felt there is no need of any database calls to assignment_questionnaires table. This contains mapping of assignment id and questionnaire id. |
| questionnaires | 311 | 0 | Leaderboard reads the scores from ScoreCache table, which has revieweeId and a response object_type (ReviewResponseMap, AuthorFeedbackResponseMap, etc.). Refactored code reuses the association of response object_type to the questionnaire_type and reduces the usage to 0. this is the most significant reduction in database calls. |
| score_caches | 22 | 1 | ScoreCache table has a field - revieweeId which can be either participantId or teamId. All repetitive database calls was reduced to a single call by combining target participantId and teamId as a list of revieweeId. |
| teams | 11 | 2 | Teams table was repetitively called within a loop to retrieve participation in a set of assignments. It was pulled out of the loop and modified to get the same result for all the teams for a larger set of assignments. |
| teams_users | 52 | 52 | No change |
| leaderboards | 9 | 5 | Leaderboards table was called multiple times to retrieve the Label for scores of corresponding questionnaire type. (e.g. "ReviewQuestionnaire" - "Submitted Work"). It was reduced to fetch all such mapping and cached. |
| courses | 5 | 5 | No change |
Performance Optimization
Google Chrome extension Page Load Time<ref> Page Load Time</ref> gives time to load a page. We tried this extension to measure performance change after refactoring. We noticed that the time to load page reduced by almost 15%.
This table indicates time to load page in seconds
| Iteration | Original Version | Refactored Version |
|---|---|---|
| 1 | 2.08 | 1.76 |
| 2 | 2.02 | 1.66 |
| 3 | 2.01 | 1.76 |
| 4 | 1.97 | 1.76 |
| 5 | 1.93 | 1.72 |
| 6 | 1.97 | 1.75 |
| 7 | 2.12 | 1.74 |
| 8 | 1.99 | 1.71 |
| 9 | 2.12 | 1.80 |
| 10 | 2.00 | 1.75 |
| Average | 2.02 | 1.74 |
Future Work
Based on use case and requirement, we can implement a metric to calculate final scores based on weights of an assignment or a questionnaire type. However, it solely depends on the the requirement whether such logic should be implemented in the Leaderboard model or the Score computation model. The team recommends that such manipulation and calculation of scores should not be part of Leaderboard model. Leaderboard model should focus on determining the eligible participants and compute the final leaderboard list.
References
<references/>