CSC/ECE 517 Fall 2014/OSS E1467 rsv: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 146: Line 146:
In below images we have shown that the output after refactoring is same. <b>We changed the order in which the output is displayed to make it more coherent.</b>
In below images we have shown that the output after refactoring is same. <b>We changed the order in which the output is displayed to make it more coherent.</b>


'''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.<br />'''e.g. Username/Password: user480/password'''


{| class="wikitable"
{| class="wikitable"

Revision as of 21:06, 29 October 2014

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 ( Reviwed by Author, Reviewed by Teammates, Submitted Work and Reviewer ), in each course. The leaderboard also shows the current standing of the logged in user under personal achievements.


Expertiza Link
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.

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.

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.
e.g. Username/Password: user480/password

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 data base 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 Dummy
users 25 25 No Change
participants 111 3 Dummy
assignments 16 5 Dummy
assignment_questionnaires 11 0 Optimized
questionnaires 311 0 Optimized
score_caches 22 1 Optimized
teams 11 2 Optimized
teams_users 52 52 No change
leaderboards 9 5 Optimized
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/>