CSC/ECE 517 Fall 2014/OSS E1467 rsv: Difference between revisions
Line 182: | Line 182: | ||
| 2.08 | | 2.08 | ||
| 1.76 | | 1.76 | ||
|- | |- | ||
| 2.02 | | 2.02 | ||
Line 189: | Line 188: | ||
| 2.01 | | 2.01 | ||
| 1.76 | | 1.76 | ||
|- | |- | ||
| 1.97 | | 1.97 | ||
| 1.76 | | 1.76 | ||
|- | |- | ||
| 1.93 | | 1.93 | ||
| 1.72 | | 1.72 | ||
|- | |- | ||
| 1.97 | | 1.97 | ||
| 1.75 | | 1.75 | ||
|- | |- | ||
| 2.12 | | 2.12 | ||
| 1.74 | | 1.74 | ||
|- | |- | ||
| 1.99 | | 1.99 | ||
| 1.71 | | 1.71 | ||
|- | |- | ||
| 2.12 | | 2.12 | ||
| 1.80 | | 1.80 | ||
|- | |- | ||
| 2.00 | | 2.00 |
Revision as of 19:26, 29 October 2014
Introduction
Project name: E1467: Expertiza - Refactoring LeaderBoard model
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 types, in each courses. 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 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.
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 enhancement of overall complexity of the feature.
Changes In 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 redundantcode computation and made the logic easier to understand. The overall complexity of this method is reduced from 72 to 35. |
5 | addEntryToCSHash
|
Confusing code with extra data base calls. | The new method name is addScoreToResultantHash . As mentioned earlier, this is called internally
by |
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 (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. | This method was refactored to reduce multiple database calls
and remove unnecessary loops. 1 database call outside and 1 within a loop was replaced by a 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
Refactored Instance : http://152.1.13.181:3000
Original Instance : http://152.1.13.181:3001
On VCL session there are 2 instances running. On port 3001
the orignal 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.
View of Refactored Leaderboard | View of Original Leaderboard |
---|---|
|
Optimization
The refactoring process included
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.
The report shows that all the 3 methods to be refactored have improved code complexity by over 50%.
Performance Optimization
Original Version | Refacatored Version |
---|---|
2.08 | 1.76 |
2.02 | 1.66 |
2.01 | 1.76 |
1.97 | 1.76 |
1.93 | 1.72 |
1.97 | 1.75 |
2.12 | 1.74 |
1.99 | 1.71 |
2.12 | 1.80 |
2.00 | 1.75 |
Future Work
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.
Lastly, we would like to recommend the readers 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. user480/password
References
<references/>