CSC/ECE 517 Spring 2015/oss E1503 RSA: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
 
(33 intermediate revisions by 3 users not shown)
Line 1: Line 1:
'''E1503. Refactor Leaderboard model, Leaderboard_helper and LeaderboardController classes'''
'''E1503. Refactor Leaderboard model, Leaderboard_helper and LeaderboardController classes'''<ref>https://docs.google.com/document/d/1J0WUBtYV_MDhpEQ-50z8gXE-OrvVkpEaZxvn_9RCAsM/edit?pli=1</ref>


This page provides a description of the Expertiza based OSS project. This project aimed at refactoring the Leaderboard model, LeaderboardController, and Leaderboard_helper classes as per standard coding methodology for Ruby on Rails.
This page provides a description of the Expertiza based OSS project. This project aimed at refactoring the Leaderboard model, LeaderboardController, and Leaderboard_helper classes as per standard coding methodology for Ruby on Rails.
Line 12: Line 12:
__TOC__
__TOC__


= Problem Statement =
= Project Description =
'''Classes involved:'''
'''Classes involved:'''
  bookmark.rb
  leaderboard.rb
  bmapping.rb
  leaderboard_controller.rb
bookmarks_controller.rb
auth_controller.rb
auth_helper.rb


'''What they do:'''
'''Modules involved: '''
The bookmark model and controller help in creating user specific bookmarks, which can be added to topics. Also, they provide the functionality to edit these bookmarks by the created user. User can search bookmarks by either users who created them or the bookmark tags.
leaderboard_helper.rb


'''What needs to be done:'''
'''What they do'''
The search methods in bookmarks model are being used used in a very granular level. This led to redundancy in bookmarks search methods. These methods include    <i>search_alltags_allusers, search_fortags_allusers, search_fortags_forusers, search_alltags_forusers</i>. Hence duplicacies in these methods are to be singled out. As we know, in ruby, the method name should specify the pseudo use of itself. Here, <i>add_new_bookmark </i> misleads as this is one of the CRUB method and the naming convention is not quite right. It has to be renamed to create method. Again the case of duplicates arrive. Now in the methods <i> add_topic_bookmark, add_this_bookmark, add_bookmark </i>. The difference between <i> add_topic_bookmark and add_this_bookmark </i> is that the former takes an extra input i.e. topicid. Hence these two can be moved into one method. The third method is an unnecessary repetition of the former two methods' functionality. The method name <i> edit_this_bookmark </i> should be renamed to <i> edit</i> along with updating all the dependencies corresponding to <i> edit_this_bookmark </i>.Lastly, <i> add_bmapping and add_bmapping_signuptopic </i> have to be moved to appropriate class i.e. bmapping class. This involves checking their dependencies and updating their function calls accordingly.
<p>These class are responsible for calculating top 3 individuals which is to be displayed as the leaderboard for the class and generate a metric which aggregates peer review scores for all course assignments and then sorts individuals.</p>


=Changes Made=
'''What needs to be done'''
<p>Methods like <i>getParticipantsScore</i> and <i>extractPersonalAchievements</i> needs to be refactored as these single functions have multiple responsibilities. They can be modularized delegating single resposibility to single method. <i>sortHash</i> method is not an explicit leaderboard model and can be moved to helper methods. Some snippets of code are redundant and have no effect on functionality. They can be removed.</p>


=Changes Made<ref>https://github.com/rkyadav-ncsu/expertiza/commits/rails4</ref>=
Leaderboard model was having public methods containing more than one feature in each method. We refactored such public methods in necessary public and private methods. Since we didn't create any new public method, we used the existing test cases to validate the changes. We removed helper methods from model class to respective helper class and changed all the references in model and controller classes.
==Leaderboard Model==
==Leaderboard Model==
{| class="wikitable"
{| class="wikitable"
Line 50: Line 50:
| Removed unwanted code snippets
| Removed unwanted code snippets
| Redundant code snippet was found which has no affect on program.
| Redundant code snippet was found which has no affect on program.
|-
| model/Leaderboard#extractPersonalAchievements
|Refactored method functionality and created private method, getCourseAccomplishmentHash
|More than one feature was implemented into one method. we split the functionality and remove code redundancy.
|-
|-
|}
|}


==Bookmark Controller==
==Leaderboard Controller==
{| class="wikitable"
{| class="wikitable"
|-
|-
Line 60: Line 64:
! style="width:43%;"|Reason For Change
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
|- style="vertical-align:top;"
| create
| index
| ----------------
| Updated reference to sortHash method
| -----------
| We moved sortHash method from model class to helper class. We updated all the old references for this method in index method on mentioned controller.
|-
| update
| -----------------
| --------
|-
| edit
| --------------
| ---------
|-
| rowspan="3" valign="middle" |
destroy
| -----------------
| ---------
|-
| ---------------
| ----------
|-
| ----------
| ----------
|}
==Views==
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| bookmarks/_result.html.erb
| ----------------
| -----------
|-
| bookmarks/search_bookmarks.html.erb
| -----------------
| --------
|-
| bookmarks/view_bookmarks.html.erb
| --------------
| ---------
|-
| bookmarks/_searchmine.html.erb
| -----------------
| ---------
|-
| bookmarks/add_bookmark_form.html.erb
| -----------------
| ---------
|-
| bookmarks/edit_bookmark_form.html.erb
| -----------------
| ---------
|-
| bookmarks/managing_bookmarks.html.erb
| -----------------
| ---------
|-
| layouts/application.html.erb
| -----------------
| ---------
|}
|}


= Re-factored Code Cases =
= Re-factored Code Cases =


== Case 1 : Refactoring add_this_bookmark and add_topic_bookmark ==
==Changes made to leaderboard_model.rb==


The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.  
===Methods added to leaderboard_model.rb===


===Before Changes===
<pre>
  # This method returns the unique participants for assignment list.
  def self.getAssignmentUniqueParticipantList(assignmentList)
    AssignmentParticipant.where(:parent_id => assignmentList.pluck(:id)).uniq
  end
 
  # This method returns the unique participant teams for assignment list.
  def self.getAssignmentUniqueParticipantTeamList(assignmentList)
    Team.where("parent_id IN (?) AND type = ?", assignmentList.pluck(:id), 'AssignmentTeam').uniq
  end
</pre>
 
===Before Refactoring===
<pre>
# Get all participants of the assignment list
participantList = AssignmentParticipant.where(:parent_id => assignmentList.pluck(:id)).uniq
 
# Get all teams participated in the given assignment list.
teamList = Team.where("parent_id IN (?) AND type = ?", assignmentList.pluck(:id), 'AssignmentTeam').uniq
assignmentMap = getAssignmentMapping(assignmentList, participantList, teamList)
</pre>
 
===After Refactoring===
<pre>
assignmentMap = getAssignmentMapping(assignmentList, getAssignmentUniqueParticipantList(assignmentList), getAssignmentUniqueParticipantTeamList(assignmentList))
</pre>
 
=== Private Methods added to leaderboard_model.rb===


<pre>
<pre>
   def self.add_topic_bookmark(b_url, b_title, b_tags_text, b_description,session_user, topicid)
  #All the methods below are private methods
     # Check if the bookmark exists and add / edit based on that
  private
    bookmark_exists = check_bookmark_exists(b_url)
  #End result is a hash (qType => (course => (user => score)))
    bmapping_exists = check_bmapping_exists(b_url,session_user)
   def Self.getUserScoreHashForCourse(scores,qTypeHash)
    if (!bookmark_exists || !bmapping_exists)
     for scoreEntry in scores
       Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user,topicid)
      revieweeUserIdList = Array.new
    elsif (bmapping_exists)
      if(assignmentMap["team"].has_key?(scoreEntry.reviewee_id))
       Bookmark.edit_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
        # Reviewee is a team. Actual Reviewee will be users of the team.
        teamUserIds = TeamsUser.where(:team_id => scoreEntry.reviewee_id).pluck(:user_id)
        revieweeUserIdList.concat(teamUserIds)
        courseId = assignmentMap["team"][scoreEntry.reviewee_id].try(:course_id).to_i
       else
        # Reviewee is an individual participant.
        revieweeUserIdList << assignmentMap["participant"][scoreEntry.reviewee_id]["self"].try(:user_id)
        courseId = assignmentMap["participant"][scoreEntry.reviewee_id]["assignment"].try(:course_id).to_i
       end
 
      questionnaireType = questionnaireResponseTypeHash[scoreEntry.object_type]
 
      addScoreToResultantHash(qTypeHash, questionnaireType, courseId, revieweeUserIdList, scoreEntry.score)
     end
     end
    qTypeHash
  end
  def Self.getRevieweeListScore(revieweeList,questionnaireResponseTypeHash)
    ScoreCache.where("reviewee_id IN (?) and object_type IN (?)", revieweeList, questionnaireResponseTypeHash.keys)
  end
  # Aggregate total reviewee list for an assignment
  def Self.getAggregatedAssignmentRevieweeList(assignmentList)
    revieweeList=Array.new
    revieweeList= getAssignmentUniqueParticipantTeamList(assignmentList).pluck(:id)
    revieweeList.concat(getAssignmentUniqueParticipantTeamList(assignmentList).pluck(:id)).uniq!
  end
  # This method returns the unique participants for assignment list.
  def self.getAssignmentUniqueParticipantList(assignmentList)
    AssignmentParticipant.where(:parent_id => assignmentList.pluck(:id)).uniq
  end
  # This method returns the unique participant teams for assignment list.
  def self.getAssignmentUniqueParticipantTeamList(assignmentList)
    Team.where("parent_id IN (?) AND type = ?", assignmentList.pluck(:id), 'AssignmentTeam').uniq
  end
  #All the methods till here are private methods
</pre>
===New method added to leaderboard_helper.rb===
<pre>
  # This method does a destructive sort on the computed scores hash so
  # that it can be mined for personal achievement information
  def self.sortHash(qTypeHash)
    result = Hash.new
    # Deep-copy of Hash
    result = Marshal.load(Marshal.dump(qTypeHash))
    result.each { |qType, courseHash|
      courseHash.each { |courseId, userScoreHash|
        userScoreSortArray = userScoreHash.sort { |a, b| b[1][0] <=> a[1][0]}
        result[qType][courseId] = userScoreSortArray
      }
    }
    result
   end
   end
</pre>


   def self.add_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
===Method removed from leaderboard_model.rb===
    bookmark_exists = check_bookmark_exists(b_url)
 
    bmapping_exists = check_bmapping_exists(b_url,session_user)
<pre>
    if (!bookmark_exists || !bmapping_exists)
  # This method does a destructive sort on the computed scores hash so
      Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
  # that it can be mined for personal achievement information
     elsif (bmapping_exists)
   def self.sortHash(qTypeHash)
       Bookmark.edit_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
    result = Hash.new
    # Deep-copy of Hash
    result = Marshal.load(Marshal.dump(qTypeHash))
 
    result.each { |qType, courseHash|
      courseHash.each { |courseId, userScoreHash|
        userScoreSortArray = userScoreHash.sort { |a, b| b[1][0] <=> a[1][0]}
        result[qType][courseId] = userScoreSortArray
      }
    }
    result
  end
</pre>
 
===Before Refactoring===
 
<pre>
  def self.addScoreToResultantHash(qTypeHash, questionnaireType, courseId, revieweeUserIdList, scoreEntryScore)
 
  userHash[revieweeUserId] = [scoreEntryScore, 1]
 
  qTypeHash[questionnaireType][courseId][revieweeUserId] = [scoreEntryScore, 1]
 
  currentUserScore[0] = (currentTotalScore + scoreEntryScore) / currentUserScore[1]
 
  csSortedHash = Leaderboard.sortHash(csHash)
</pre>
 
===After Refactoring===
 
<pre>
  def self.addScoreToResultantHash(qTypeHash, questionnaireType, courseId, revieweeUserIdList, entryScore)
 
  userHash[revieweeUserId] = [entryScore, 1]
 
  qTypeHash[questionnaireType][courseId][revieweeUserId] = [entryScore, 1]
 
  currentUserScore[0] = (currentTotalScore + entryScore) / currentUserScore[1]
 
  csSortedHash = LeaderboardHelper.sortHash(csHash)
</pre>
 
==Changes made to leaderboard_helper.rb==
 
===New method added to leaderboard_helper.rb===
 
<pre>
# This method returns course accomplishment hash for a user
  def self.getCourseAccomplishmentHash(courseIdList,accomplishmentMap,userId,csHash)
     courseAccomplishmentHash = Hash.new
    csSortedHash = LeaderboardHelper.sortHash(csHash)
 
    for courseId in courseIdList
       for accomplishment in accomplishmentMap.keys
        # Get score for current questionnaireType/accomplishment, courseId and userId from csHash
        score = csHash.fetch(accomplishment, {}).fetch(courseId, {}).fetch(userId, nil)
        if(score)
          if courseAccomplishmentHash[courseId].nil?
            courseAccomplishmentHash[courseId] = Array.new
          end
          # Calculate rank of current user
          rank = 1 + csSortedHash[accomplishment][courseId].index([userId, score])
          total = csSortedHash[accomplishment][courseId].length
 
          courseAccomplishmentHash[courseId] << {:accomp => accomplishmentMap[accomplishment],
                                                :score => score[0],
                                                :rankStr => "#{rank} of #{total}"
          }
        end
      end
     end
     end
    courseAccomplishmentHash
   end
   end
</pre>
</pre>


===After Changes===
===Before Refactoring===
 
<pre>
<pre>
  def self.add_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user, topic_id)
 
     bookmark_exists = check_bookmark_exists(b_url)
    csSortedHash = LeaderboardHelper.sortHash(csHash)
    bmapping_exists = check_bmapping_exists(b_url,session_user)
 
    if (!bookmark_exists || !bmapping_exists)
     for courseId in courseIdList
      if(topic_id) # something has been passed for topic_id
      for accomplishment in accomplishmentMap.keys
        Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user,topic_id)
        # Get score for current questionnaireType/accomplishment, courseId and userId from csHash
      else
        score = csHash.fetch(accomplishment, {}).fetch(courseId, {}).fetch(userId, nil)
         Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user, nil)
        if(score)
          if courseAccomplishmentHash[courseId].nil?
            courseAccomplishmentHash[courseId] = Array.new
          end
          # Calculate rank of current user
          rank = 1 + csSortedHash[accomplishment][courseId].index([userId, score])
          total = csSortedHash[accomplishment][courseId].length
 
          courseAccomplishmentHash[courseId] << {:accomp => accomplishmentMap[accomplishment],
                                                :score => score[0],
                                                :rankStr => "#{rank} of #{total}"
          }
         end
       end
       end
    elsif (bmapping_exists)
      Bookmark.edit(b_url, b_title, b_tags_text, b_description,session_user)
     end
     end
  end
    courseAccomplishmentHash
 
</pre>
</pre>


== Case 2 :  ==
===After Refactoring===
 
<pre>
  return getCourseAccomplishmentHash(courseIdList,accomplishmentMap,userId,csHash)
</pre>


=Steps to verify changes manually=
==Changes made to leaderboard_controller.rb==


===User2 Role===
===Before===


=Automated Tests=
<pre>
@csHash = Leaderboard.sortHash(@csHash)
</pre>


===Unit test===
===After===


===Integration (Cucumber) tests===
<pre>
@csHash = LeaderboardHelper.sortHash(@csHash)
</pre>


=See Also=
=Automated Tests=


===Unit test===
We didn't create any new public method as part of this project, instead we refactored the existing code. Existing test suit was suitable for testing our modifications.


= References=
= References=
<references/>

Latest revision as of 00:56, 24 March 2015

E1503. Refactor Leaderboard model, Leaderboard_helper and LeaderboardController classes<ref>https://docs.google.com/document/d/1J0WUBtYV_MDhpEQ-50z8gXE-OrvVkpEaZxvn_9RCAsM/edit?pli=1</ref>

This page provides a description of the Expertiza based OSS project. This project aimed at refactoring the Leaderboard model, LeaderboardController, and Leaderboard_helper classes as per standard coding methodology for Ruby on Rails.

Introduction to Expertiza

Expertiza is a peer review based system used to provide improved learning experience. Project is developed as a combined effort of students and faculty using the Ruby on Rails framework. Expertiza allows the instructor to create and customize assignments, create a list of topics the students can sign up for, have students work on teams and then review each other's assignments at the end. Expertiza supports submission of almost any document type, including the URLs and wiki pages.

Leaderboard is a module that can be used to find top three leaders in any class based on the score they have received on their submissions and reviews.


Project Description

Classes involved:

leaderboard.rb
leaderboard_controller.rb

Modules involved:

leaderboard_helper.rb

What they do

These class are responsible for calculating top 3 individuals which is to be displayed as the leaderboard for the class and generate a metric which aggregates peer review scores for all course assignments and then sorts individuals.

What needs to be done

Methods like getParticipantsScore and extractPersonalAchievements needs to be refactored as these single functions have multiple responsibilities. They can be modularized delegating single resposibility to single method. sortHash method is not an explicit leaderboard model and can be moved to helper methods. Some snippets of code are redundant and have no effect on functionality. They can be removed.

Changes Made<ref>https://github.com/rkyadav-ncsu/expertiza/commits/rails4</ref>

Leaderboard model was having public methods containing more than one feature in each method. We refactored such public methods in necessary public and private methods. Since we didn't create any new public method, we used the existing test cases to validate the changes. We removed helper methods from model class to respective helper class and changed all the references in model and controller classes.

Leaderboard Model

Method Name Changes Made Reason For Change
model/Leaderboard#getParticipantsScore Refactored original method functionality into
getAssignmentUniqueParticipantList,
getAssignmentUniqueParticipantTeamList,
getAggregatedAssignmentRevieweeList,
getRevieweeListScore, and
getUserScoreHashForCourse methods
Many different features were originally written in one method. We refactored this method into many methods to achieve singularity.
model/Leaderboard#sortHash
helpers/leaderboard_helper#sortHash
Migrated method from Leaderboard model to leaderboard_helper class. sortHash is a helper method and is not explicit part of model class, hence it was moved to achieve code reusability and remove redundancy.
model/Leaderboard scoreEntryScore parameter Parameter name is changed to entryScore The name was confusing. It was used to represent each entry's score, hence we changed the name to maintain readability of code.
model/Leaderboard#extractPersonalAchievements Removed unwanted code snippets Redundant code snippet was found which has no affect on program.
model/Leaderboard#extractPersonalAchievements Refactored method functionality and created private method, getCourseAccomplishmentHash More than one feature was implemented into one method. we split the functionality and remove code redundancy.

Leaderboard Controller

Method Name Changes Made Reason For Change
index Updated reference to sortHash method We moved sortHash method from model class to helper class. We updated all the old references for this method in index method on mentioned controller.

Re-factored Code Cases

Changes made to leaderboard_model.rb

Methods added to leaderboard_model.rb

  # This method returns the unique participants for assignment list.
  def self.getAssignmentUniqueParticipantList(assignmentList)
    AssignmentParticipant.where(:parent_id => assignmentList.pluck(:id)).uniq
  end

  # This method returns the unique participant teams for assignment list.
  def self.getAssignmentUniqueParticipantTeamList(assignmentList)
    Team.where("parent_id IN (?) AND type = ?", assignmentList.pluck(:id), 'AssignmentTeam').uniq
  end

Before Refactoring

# Get all participants of the assignment list
participantList = AssignmentParticipant.where(:parent_id => assignmentList.pluck(:id)).uniq

# Get all teams participated in the given assignment list.
teamList = Team.where("parent_id IN (?) AND type = ?", assignmentList.pluck(:id), 'AssignmentTeam').uniq
assignmentMap = getAssignmentMapping(assignmentList, participantList, teamList)

After Refactoring

assignmentMap = getAssignmentMapping(assignmentList, getAssignmentUniqueParticipantList(assignmentList), getAssignmentUniqueParticipantTeamList(assignmentList))

Private Methods added to leaderboard_model.rb

  #All the methods below are private methods
  private
  #End result is a hash (qType => (course => (user => score)))
  def Self.getUserScoreHashForCourse(scores,qTypeHash)
    for scoreEntry in scores
      revieweeUserIdList = Array.new
      if(assignmentMap["team"].has_key?(scoreEntry.reviewee_id))
        # Reviewee is a team. Actual Reviewee will be users of the team.
        teamUserIds = TeamsUser.where(:team_id => scoreEntry.reviewee_id).pluck(:user_id)
        revieweeUserIdList.concat(teamUserIds)
        courseId = assignmentMap["team"][scoreEntry.reviewee_id].try(:course_id).to_i
      else
        # Reviewee is an individual participant.
        revieweeUserIdList << assignmentMap["participant"][scoreEntry.reviewee_id]["self"].try(:user_id)
        courseId = assignmentMap["participant"][scoreEntry.reviewee_id]["assignment"].try(:course_id).to_i
      end

      questionnaireType = questionnaireResponseTypeHash[scoreEntry.object_type]

      addScoreToResultantHash(qTypeHash, questionnaireType, courseId, revieweeUserIdList, scoreEntry.score)
    end
    qTypeHash
  end

  def Self.getRevieweeListScore(revieweeList,questionnaireResponseTypeHash)
    ScoreCache.where("reviewee_id IN (?) and object_type IN (?)", revieweeList, questionnaireResponseTypeHash.keys)
  end
  # Aggregate total reviewee list for an assignment
  def Self.getAggregatedAssignmentRevieweeList(assignmentList)
    revieweeList=Array.new
    revieweeList= getAssignmentUniqueParticipantTeamList(assignmentList).pluck(:id)
    revieweeList.concat(getAssignmentUniqueParticipantTeamList(assignmentList).pluck(:id)).uniq!

  end
  # This method returns the unique participants for assignment list.
  def self.getAssignmentUniqueParticipantList(assignmentList)
    AssignmentParticipant.where(:parent_id => assignmentList.pluck(:id)).uniq
  end

  # This method returns the unique participant teams for assignment list.
  def self.getAssignmentUniqueParticipantTeamList(assignmentList)
    Team.where("parent_id IN (?) AND type = ?", assignmentList.pluck(:id), 'AssignmentTeam').uniq
  end
  #All the methods till here are private methods

New method added to leaderboard_helper.rb

  # This method does a destructive sort on the computed scores hash so
  # that it can be mined for personal achievement information
  def self.sortHash(qTypeHash)
    result = Hash.new
    # Deep-copy of Hash
    result = Marshal.load(Marshal.dump(qTypeHash))

    result.each { |qType, courseHash|
      courseHash.each { |courseId, userScoreHash|
        userScoreSortArray = userScoreHash.sort { |a, b| b[1][0] <=> a[1][0]}
        result[qType][courseId] = userScoreSortArray
      }
    }
    result
  end

Method removed from leaderboard_model.rb

  # This method does a destructive sort on the computed scores hash so
  # that it can be mined for personal achievement information
  def self.sortHash(qTypeHash)
    result = Hash.new
    # Deep-copy of Hash
    result = Marshal.load(Marshal.dump(qTypeHash))

    result.each { |qType, courseHash|
      courseHash.each { |courseId, userScoreHash|
        userScoreSortArray = userScoreHash.sort { |a, b| b[1][0] <=> a[1][0]}
        result[qType][courseId] = userScoreSortArray
      }
    }
    result
  end

Before Refactoring

  def self.addScoreToResultantHash(qTypeHash, questionnaireType, courseId, revieweeUserIdList, scoreEntryScore)

  userHash[revieweeUserId] = [scoreEntryScore, 1]

  qTypeHash[questionnaireType][courseId][revieweeUserId] = [scoreEntryScore, 1]

  currentUserScore[0] = (currentTotalScore + scoreEntryScore) / currentUserScore[1]

  csSortedHash = Leaderboard.sortHash(csHash)

After Refactoring

  def self.addScoreToResultantHash(qTypeHash, questionnaireType, courseId, revieweeUserIdList, entryScore)

  userHash[revieweeUserId] = [entryScore, 1]

  qTypeHash[questionnaireType][courseId][revieweeUserId] = [entryScore, 1]

  currentUserScore[0] = (currentTotalScore + entryScore) / currentUserScore[1]

  csSortedHash = LeaderboardHelper.sortHash(csHash)

Changes made to leaderboard_helper.rb

New method added to leaderboard_helper.rb

 # This method returns course accomplishment hash for a user
  def self.getCourseAccomplishmentHash(courseIdList,accomplishmentMap,userId,csHash)
    courseAccomplishmentHash = Hash.new
    csSortedHash = LeaderboardHelper.sortHash(csHash)

    for courseId in courseIdList
      for accomplishment in accomplishmentMap.keys
        # Get score for current questionnaireType/accomplishment, courseId and userId from csHash
        score = csHash.fetch(accomplishment, {}).fetch(courseId, {}).fetch(userId, nil)
        if(score)
          if courseAccomplishmentHash[courseId].nil?
            courseAccomplishmentHash[courseId] = Array.new
          end
          # Calculate rank of current user
          rank = 1 + csSortedHash[accomplishment][courseId].index([userId, score])
          total = csSortedHash[accomplishment][courseId].length

          courseAccomplishmentHash[courseId] << {:accomp => accomplishmentMap[accomplishment],
                                                 :score => score[0],
                                                 :rankStr => "#{rank} of #{total}"
          }
        end
      end
    end
    courseAccomplishmentHash
  end

Before Refactoring


    csSortedHash = LeaderboardHelper.sortHash(csHash)

    for courseId in courseIdList
      for accomplishment in accomplishmentMap.keys
        # Get score for current questionnaireType/accomplishment, courseId and userId from csHash
        score = csHash.fetch(accomplishment, {}).fetch(courseId, {}).fetch(userId, nil)
        if(score)
          if courseAccomplishmentHash[courseId].nil?
            courseAccomplishmentHash[courseId] = Array.new
          end
          # Calculate rank of current user
          rank = 1 + csSortedHash[accomplishment][courseId].index([userId, score])
          total = csSortedHash[accomplishment][courseId].length

          courseAccomplishmentHash[courseId] << {:accomp => accomplishmentMap[accomplishment],
                                                 :score => score[0],
                                                 :rankStr => "#{rank} of #{total}"
          }
        end
      end
    end
    courseAccomplishmentHash

After Refactoring

  return getCourseAccomplishmentHash(courseIdList,accomplishmentMap,userId,csHash)

Changes made to leaderboard_controller.rb

Before

@csHash = Leaderboard.sortHash(@csHash)

After

@csHash = LeaderboardHelper.sortHash(@csHash)

Automated Tests

Unit test

We didn't create any new public method as part of this project, instead we refactored the existing code. Existing test suit was suitable for testing our modifications.

References

<references/>