CSC/ECE 517 Fall 2014/OSS E1467 rsv

From Expertiza_Wiki
Jump to navigation Jump to search

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.

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

In the OSS project, we have refactored these methods along with other smaller methods in Leaderboard.rb , Leaderboard_helper.rb , Leaderboard_controller.rb and associated views files. We have also refactored ambiguous variable and method names.

We have keenly focussed in reducing the database calls, loops and redundant storage and computation. We have refactored many files related to Leaderboard implementation leading to reduction in overall complexity of the feature.

Also, there was a requirement in the project, that we should come up with an efficient way to search for participants based on assignment ids. However, upon investigation, we found that scores stored in table ScoreCache, gives us revieweeId which is either participantId or teamId. Therefore, we have a method getAssignmentMapping , which creates a mapping of participant and team with corresponding assignment. This is very useful while computing leaderboard.

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 Lleaderboard 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 by the database driver by filtering the messages from the rails server's log. In the original code there were 625 database select access generated for the leaderboard view. In the new refactored 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/>

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2014/OSS_E1467_rsv&oldid=90973"