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

From Expertiza_Wiki
Jump to navigation Jump to search
 
(143 intermediate revisions by 4 users not shown)
Line 1: Line 1:
=Expertiza - Refactoring LeaderBoard model=
=Introduction=
'''Project name: [https://docs.google.com/document/d/1FZCL9KWSdVNsX9BowuZ3gxbCOJoiWX-GVLctSZei3No/edit E1467: Expertiza - Refactoring LeaderBoard model]'''<ref>[https://docs.google.com/document/d/1FZCL9KWSdVNsX9BowuZ3gxbCOJoiWX-GVLctSZei3No/edit E1467: Expertiza - Refactoring LeaderBoard model]</ref>
 
Our project is to refactor the code in the ''"Leaderboard"'' functionality of the web application [https://github.com/expertiza/expertiza Expertiza]<ref>[https://github.com/expertiza/expertiza 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.
 
<div id="projectLinks" style="float: right;">
[[File:expertiza_logo.jpg|center]]
{| class="wikitable"
| colspan="3" align="center" | '''Expertiza project links'''
|-
| '''Refactored Instance'''
| http://152.46.18.78:3000/
| rowspan="2" | '''username= "user480"<br />password= "password"'''
|-
| '''Original Instance'''
| http://152.46.18.78:3001/
|-
| '''Repository link'''
| colspan="3" | https://github.com/sanjeevs/expertiza <ref>[https://github.com/sanjeevs/expertiza Forked repository]</ref>
|-
| '''Pull request'''
| colspan="3" | https://github.com/expertiza/expertiza/pull/444 <ref>[https://github.com/expertiza/expertiza/pull/444 Pull request]</ref>
|}
</div>
 
__TOC__
__TOC__


==Introduction==
=Project Description=
Our project is to refactor the code in the "Leaderboard" functionality of the web application Expertiza<ref name="expertiza>''Expertiza'' http://wikis.lib.ncsu.edu/index.php/Expertiza</ref>. The Expertiza project is a system to create reusable learning objects through peer review. The classes involved gets all the assignments in the course and all the participants in a course or in an assignment. The responsibility of the leaderboard module is to generate the top 3 individuals which is to be displayed as the leaderboard for the class and a personal leaderboard view to see a personal aggregated score.
'''Classes involved:''' ''leaderboard.rb'' and associated other model and controllers classes.  
 
==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.
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.
2. Refactor score hash for personal achievements. (method: <code style="background:#F0F0FF"> extractPersonalAchievements </code>). Seperate out method which will rank individual personal achievements.


3. Refactor addEntryToCSHash according to your new metric method.
3. Refactor <code style="background:#F0F0FF"> addEntryToCSHash </code> according to your new metric method.


4. Seperate out computation of CS entries(metric) and refactor it to be more modular and elegant.  
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).
5. Refactor <code style="background:#F0F0FF"> getParticipantEntriesInAssignmentList </code> method. Its very complex <code style="background:#F0F0FF">(Complexity=142)</code>.


6. Refactor addEntryToCSHash according to your new metric method.
6. Refactor <code style="background:#F0F0FF"> addEntryToCSHash </code> according to your new metric method.


==Existing Functionality==
=Existing Functionality=
Leaderboard class gets all the assignments within all the courses taken by currently logged in user. It
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''.
also fetches any independent assignments (not associated with any course), that the user has taken.


Leaderboard model has following 3 important methods:
Leaderboard model has following 3 important methods:
{| class="wikitable"
{| class="wikitable"
|-
|-
! style="width:5%;"|Sr. No.
! style="width:2%;"|Sr. No.
! style="width:13%;"|Method Name
! style="width:13%;"|Method Name
! style="width:43%;"|Comment
! style="width:43%;"|Comment
|- style="vertical-align:top;"
|- style="vertical-align:top;"
|''''' 1 '''''
| getParticipantEntriesInAssignmentList(assignmentList)
|This method is responsible for calculating the leaderboard of all the participants associated
with assignments in given assignment list.
|-
|-
|''''' 2 '''''
|style="text-align:center;"|''''' 1 '''''
| extractPersonalAchievements(csHash, courseIdList, userId)
| <code> getParticipantEntriesInAssignmentList(assignmentList) </code>
| This method is responsible for calculating personal aggregated score. It also calculates the
|This method is responsible for calculating the leaderboard of all the participants associated with assignments in given assignment list.
ranking of currently logged in user against total users associated with the same set of
courses as that of current user.
|-
|-
|''''' 3 '''''
|style="text-align:center;"|''''' 2 '''''
| addEntryToCSHash(qtypeHash, qtype, userid, csEntry, courseid)
| <code> extractPersonalAchievements(csHash, courseIdList, userId) </code>
| This method is called internally from Leaderboard.getParticipantEntriesInAssignmentList. This method aggregates score of a user grouped by course id, further grouped by
| 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.
questionnaire type.
|-
|style="text-align:center;"|''''' 3 '''''
| <code> addEntryToCSHash(qtypeHash, qtype, userid, csEntry, courseid) </code>
| This method is called internally from <code style="background:#F0F0FF"> Leaderboard.getParticipantEntriesInAssignmentList </code>. 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
In the OSS project, we have refactored these methods along with other smaller methods in
Leaderboard.rb, Leaderboard_helper.rb. We have also refactored ambiguous variable and method
<code style="background:#F0F0FF"> Leaderboard.rb </code>, <code style="background:#F0F0FF"> Leaderboard_helper.rb </code>, <code style="background:#F0F0FF"> Leaderboard_controller.rb </code> and associated views files. We have also refactored ambiguous variable and method names.
names.


We have keenly focussed in reducing the database calls, loops and redundant storage and
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.
computation.  
 
We have refactored many files related to Leaderboard implementation leading to
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 <code> getAssignmentMapping </code>, which creates a mapping of participant and team with corresponding assignment. This is very useful while computing leaderboard.
enhancement of overall complexity of the feature.
 
Changes made in the methods of model <code> '''leaderboard.rb''' </code>
{| class="wikitable"
|-
! style="width:2%;"|Sr. No.
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
|style="text-align:center;"|''''' 1 '''''
| <code> getIndependantAssignments </code>
| Separate db call for each user assignment.
| Single db call with an array of assignment ids.
|-
|style="text-align:center;"|''''' 2 '''''
| <code> getParticipantEntriesInCourses </code>
| 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.
|-
|style="text-align:center;"|''''' 3 '''''
| <code> sortHash </code>
| in place changes.
| Returns a new deep copy of the updated hash.
|-
|style="text-align:center;"|''''' 4 '''''
| <code> extractPersonalAchievements </code>
| 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.
|-
|style="text-align:center;"|''''' 5 '''''
| <code> addEntryToCSHash </code>
| Confusing name and hard to follow logic.
| The new method name is <code style="background:#F0F0FF"> addScoreToResultantHash </code>. As mentioned earlier, this is called internally by <code style="background:#F0F0FF"> getParticipantEntriesInAssignmentList </code>. The complexity of this method is reduced from 67 to 39 .
|-
|style="text-align:center;"|''''' 6 '''''
| <code> getParticipantEntriesInAssignmentList </code>
| This method was the most complex method in the class.
| The new refactored method name is <code style="background:#F0F0FF"> getParticipantsScore </code>. 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 <code> leaderboard_helper.rb </code>
 
{| class="wikitable"
|-
! style="width:5%;"|Sr. No.
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
|style="text-align:center;"|''''' 1 '''''
| <code> userIsInstructor </code>
| 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.
|-
|style="text-align:center;"|''''' 2 '''''
| <code> studentInWhichCourses </code>
| 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.
|-
|style="text-align:center;"|''''' 3 '''''
| <code> getTop3Leaderboards </code>
| Dead code
| Removed.
|-
|}


==Changes==
1. Leaderboard.rb
○ Methods like getIndependantAssignments and getParticipantEntriesInCourses are
refactored to reduce the database calls within loop. Database calls have huge impact on
overall complexity of the code and performance of the software.
○ sortHash : This method modified the input hash. We refactored it to not alter the input
hash object, but rather return a new deep copy of the updated hash object.
○ extractPersonalAchievements : 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 *.
○ addEntryToCSHash : This method is refactored to reduce the redundant code. 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 *.


○ 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 *.
○ 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.
2. Leaderboard_helper.rb
○ userIsInstructor : This method was refactored to reduce multiple database calls. 4
database calls was replaced by single call.
○ studentInWhichCourses : 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.
○ getTop3Leaderboards : This method is a dead code, not called from anywhere, therefore,
we have commented this for the moment.
There are several other changes in the views and controller which deal with renaming of the methods
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
and variables, adding proper comments etc and minor code refactoring. Please refer to our forked
github repository to view those changes.
github repository to view those changes.
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
=Design Pattern=
are other existing users who have participated in the same assignment. e.g. user480/password
Leaderboard is a separate implementation which includes reading existing scores for the sole purpose of creating the leaders in different categories. As the task is more related to calculate and manipulate data, it was not feasible to implement any design pattern. We reduced the complexity and redundancy of database calls by reducing database calls from 625 to 111 to do the same task.
* Code complexity measurement is according to Code Climate
 
=Testing=
On VCL session there are 2 instances running. On <code> port 3001 </code> the original instance is setup and on <code> port 3000 </code> the refactored instance is setup. Login by any user and navigate to ''Home > Leaderboard'' to test the functionality.
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 />
[[#projectLinks|'''Expertiza Test instances links''']]
 
{| class="wikitable"
|-
! View of Refactored Leaderboard
! View of Original Leaderboard
 
|-
| [[File:Refactored_Leaderboard_file.jpg|frame|left| ]]
| [[File:Non_Refactored_Leaderboard_file.jpg|frame|left| ]]
 
|-
| [[File:Refactored_personal_achievement_file.jpg|frame|left| ]]
| [[File:Non_personal_achievement_file.jpg|frame|left| ]]
 
 
|}
 
=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. [https://codeclimate.com/ Code Climate]<ref>[https://codeclimate.com/ 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 <code style="background:#F0F0FF"> Leaderboard.rb </code> from Code Climate.
 
{| class="wikitable"
|-
! Code complexity of original leaderboard implementation
! Code complexity of refactored leaderboard implementation
|-
| [[File:Non_Refactored_Leaderboard_complexity.png|center|frame]]
| [[File:Refactored_Leaderboard_complexity.png|center|frame]]
|-
| [[File:Non_Refactored_Leaderboard_stats.png|center|frame]]
| [[File:Refactored_Leaderboard_stats.png|center|frame]]
|}
 
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.
 
[[File:CodeClimate_codecomplexity.png|center|frame]]
 
== 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.
 
{| class="wikitable"
|-
! Table Name
! Original Version
! Refactored Version
! Comment
|-
| '''roles'''
| style="text-align: center;"|52
| style="text-align: center;"|13
| Optimized the SQL query in ''leaderboard_helper.rb'' to replace multiple database calls.
|-
| '''users'''
| style="text-align: center;"|25
| style="text-align: center;"|25
| No Change
|-
| '''participants'''
| style="text-align: center;"|111
| style="text-align: center;"|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'''
| style="text-align: center;"|16
| style="text-align: center;"|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'''
| style="text-align: center;"|11
| style="text-align: center;"|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'''
| style="text-align: center;"|311
| style="text-align: center;"|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'''
| style="text-align: center;"|22
| style="text-align: center;"|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'''
| style="text-align: center;"|11
| style="text-align: center;"|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'''
| style="text-align: center;"|52
| style="text-align: center;"|52
| No change
|-
| '''leaderboards'''
| style="text-align: center;"|9
| style="text-align: center;"|5
| Leaderboards table was called multiple times to retrieve the Label for scores of corresponding questionnaire type.<br />(e.g. ''"ReviewQuestionnaire" - "Submitted Work"''). It was reduced to fetch all such mapping and cached.
|-
| '''courses'''
| style="text-align: center;"|5
| style="text-align: center;"|5
| No change
|-
|}
 
== Performance Optimization ==
 
Google Chrome extension [https://chrome.google.com/webstore/detail/page-load-time/fploionmjgeclbkemipmkogoaohcdbig?hl=en Page Load Time]<ref>
[https://chrome.google.com/webstore/detail/page-load-time/fploionmjgeclbkemipmkogoaohcdbig?hl=en 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
{| class="wikitable" style="text-align: center;"
|-
! 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
|-
|<b>Average</b>
|<b>2.02</b>
|<b>1.74</b>
|}
 
=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/>

Latest revision as of 09:35, 14 November 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 (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.

Design Pattern

Leaderboard is a separate implementation which includes reading existing scores for the sole purpose of creating the leaders in different categories. As the task is more related to calculate and manipulate data, it was not feasible to implement any design pattern. We reduced the complexity and redundancy of database calls by reducing database calls from 625 to 111 to do the same task.

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. Login by any user and navigate to Home > Leaderboard to test the functionality. 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/>