CSC/ECE 517 Spring 2015/oss E1503 RSA

E1503. Refactor Leaderboard model, Leaderboard_helper and LeaderboardController classes

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.

Problem Statement

Classes involved:

bookmark.rb
bmapping.rb
bookmarks_controller.rb
auth_controller.rb
auth_helper.rb

What they do: 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.

What needs to be done: 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 search_alltags_allusers, search_fortags_allusers, search_fortags_forusers, search_alltags_forusers. 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, add_new_bookmark 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 add_topic_bookmark, add_this_bookmark, add_bookmark . The difference between add_topic_bookmark and add_this_bookmark 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 edit_this_bookmark should be renamed to edit along with updating all the dependencies corresponding to edit_this_bookmark .Lastly, add_bmapping and add_bmapping_signuptopic have to be moved to appropriate class i.e. bmapping class. This involves checking their dependencies and updating their function calls accordingly.

Changes Made

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.

Bookmark Controller

Method Name	Changes Made	Reason For Change
create	----------------	-----------
update	-----------------	--------
edit	--------------	---------
destroy	-----------------	---------
	---------------	----------
	----------	----------

Views

Method Name	Changes Made	Reason For Change
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

Case 1 : Refactoring add_this_bookmark and add_topic_bookmark

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.

Before Changes

  def self.add_topic_bookmark(b_url, b_title, b_tags_text, b_description,session_user, topicid)
    # Check if the bookmark exists and add / edit based on that
    bookmark_exists = check_bookmark_exists(b_url)
    bmapping_exists = check_bmapping_exists(b_url,session_user)
    if (!bookmark_exists || !bmapping_exists)
      Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user,topicid)
    elsif (bmapping_exists)
      Bookmark.edit_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
    end
  end

  def self.add_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
    bookmark_exists = check_bookmark_exists(b_url)
    bmapping_exists = check_bmapping_exists(b_url,session_user)
    if (!bookmark_exists || !bmapping_exists)
      Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
    elsif (bmapping_exists)
      Bookmark.edit_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
    end
  end

After Changes

  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)
    bmapping_exists = check_bmapping_exists(b_url,session_user)
    if (!bookmark_exists || !bmapping_exists)
      if(topic_id) # something has been passed for topic_id
        Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user,topic_id)
      else
        Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user, nil)
      end
    elsif (bmapping_exists)
      Bookmark.edit(b_url, b_title, b_tags_text, b_description,session_user)
    end
  end