CSC/ECE 517 Spring 2016/Refactor review mapping controller.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
No edit summary
 
(37 intermediate revisions by 2 users not shown)
Line 1: Line 1:
{{Infobox software
This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.
| name                  =
| logo                  = <!-- Image name is enough -->
| author                =
| developer              =
| released              = <!-- {{Start date and age|YYYY|MM|DD|df=yes/no}} -->
| latest release version =
| latest release date    = <!-- {{Start date and age|YYYY|MM|DD|df=yes/no}} -->
| status                =
| programming language  =
| operating system      =
| license                =
| website                = {{URL|example.org}}
}}
Devise is a rack based full-fledged authentication system for Rails. It is a complete MVC solution meaning it can support various models, views and controllers as part of its code and can be used be directly by developers. Devise is simple to use and starts up with a couple of commands but it is also highly customizable. Devise saves a lot of time and effort as many applications require user registration and authentication mechanisms which are difficult to develop from scratch.


=='''History'''==
Devise is first introduced in January 2010 by Plataformatec, a company which builds web and mobile applications. Devise is one of the few authentication systems which support rack based applications and hence can support Rails 3 and up as they are completely rack based. The latest version of Devise available is v3.5.3 and it is up to date with Rails 5 beta 2.
===Warden===
Devise gem is built on top of a rack application called Warden which is used to verify the identity of logged in user using a session string. In Warden, the id which is a primary key of a user is somehow stored to match it later with the logged in user. Warden also provides restricted access to guest users depending on the functional requirements of an application.<br/><br/>
Since Warden does not know of the existence of the Rails application, it cannot provide any helper methods, controller classes, views etc. This is where Devise comes into play and can integrate with Rails seemlessly. Devise interacts often with Warden using Strategy design pattern for encrypting passwords, HTTP Authentication etc.


=='''Installation'''==
=='''Expertiza '''==
Although Devise is very useful and reduces the amount of effort in developing authentication mechanisms significantly, it requires a good understanding of the Rails framework. Hence it is advised for beginners to not use Devise.<br/><br/>
 
There are several commands that are required for the successful installation of Devise. They are listed out below:
The Expertiza is a software project to create reusable learning objects through peer review. It is a Ruby on Rails based application which can be run on Windows, Linux and Mac OS X.
*Add the devise gem to your gemfile.
 
gem 'devise'
Main Features of Expertiza are:
*Run bundle command to install the gem.
* Allows students to work in groups to improve others learning experiences
bundle install
* The work done by students/team is subjeted to multiple reviews , minimizing plagiarism
*You need to run the generator next which will install an initializer and creates all the configuration files.
* Reviewing is done by studnets, which allows instructor/TA to spend less time in grading
rails generate devise:install
* Large classes can also be handled easily with the help of Expertiza
*Now you can add Devise to any of your models using the generator. This will create a class with the model name given and routes etc. The model will be configured with default Devise modules. The config/routes.rb file will be configured to point to the Devise controller.
 
rails generate devise user //Assuming that the model name is user
=='''About Review mapping controller'''==
*Next, add any configuration changes that are required and then run:
 
rake db:migrate
Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.
*The following step will create Devise views but is optional. Devise has views for every generic operation like Login or SignUp and can be used directly instead of creating custom views.
 
rails generate devise:views users
==='''Problem Statement'''===
* There are also many routes that are defined in config/routes.rb with a line like:
 
devise_for :users
The main aim of this project is to
 
* Refactor reporting method(response_report) which is big
* Use more efficient function sample instead of shuffle for random number selection
* Remove unused and assigned variables
* Simplify Automatic_review_mapping_strategy
 
=='''Code Improvements'''==
 
==='''Unused variables and arguments'''===
 
There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.
 
For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replace the unused variable with "_" to represent it as unused variable but allow them in arguments.
 
<pre>
Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
</pre>
<pre>
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
</pre>
 
In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.
 
==='''Use sample instead of shuffle'''===
 
When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.
 
<pre>
Previous Code:
 
assignment_team = assignment_teams.to_a.shuffle[0] rescue nil
 
topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
</pre>
<pre>
After Changing the code:
 
assignment_team = assignment_teams.to_a.sample rescue nil
 
topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil
</pre>
 
==='''Cyclomatic complexity of automatic_review_mapping_strategy method'''===
 
The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.
 
The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.
 
The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:


=='''Modules'''==
When a devise generator is invoked a model class is created in app/models for you modify for your specific application requirements. This is the place where many important configuration changes are specified. Perhaps the most important are the Devise modules which provides essential functionalities like enhanced securtiy.<br/><br/>
There are 10 modules listed on the official page of Devise by Plataformatec. These modules are features that are contained in Devise and can be used by the developers depending on the use-cases or requirements of their application. Below is the list of modules:
*Database Authenticable
*Omniauthable
*Confirmable
*Recoverable
*Registerable
*Rememberable
*Trackable
*Timeoutable
*Validatable
*Lockable
Information regarding each module is listed in README as well as the [http://devise.plataformatec.com.br/ Plataformatec] website. The modules are included in an application in this way:
<pre>
<pre>
class User < ActiveRecord::Base
private
  devise :database_authenticable, :omniauthable, :confirmable, :rememberable,
def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
          :trackable, :timeoutable, :lockable
iterator = 0
end
teams.each do |team|
  selected_participants = Array.new
  if !team.equal? teams.last
#need to even out the # of reviews for teams
while selected_participants.size < num_reviews_per_team
  num_participants_this_team = TeamsUser.where(team_id: team.id).size
  #If there are some submitters or reviewers in this team, they are not treated as normal participants.
  #They should be removed from 'num_participants_this_team'
  TeamsUser.where(team_id: team.id).each do |team_user|
temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
  end
  #if all outstanding participants are already in selected_participants, just break the loop.
  break if selected_participants.size == participants.size - num_participants_this_team
 
  # generate random number
  if iterator == 0
rand_num = rand(0..num_participants-1)
  else
min_value = participants_hash.values.min
#get the temp array including indices of participants, each participant has minimum review number in hash table.
participants_with_min_assigned_reviews = Array.new
participants.each do |participant|
  participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
end
#if participants_with_min_assigned_reviews is blank
if_condition_1 = participants_with_min_assigned_reviews.empty?
#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
if if_condition_1 or if_condition_2
  #use original method to get random number
  rand_num = rand(0..num_participants-1)
else
  #rand_num should be the position of this participant in original array
  rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
end
  end
  # prohibit one student to review his/her own artifact
  next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)
 
  if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
  if_condition_2 = (!selected_participants.include? participants[rand_num].id)
  if if_condition_1 and if_condition_2
# selected_participants cannot include duplicate num
selected_participants << participants[rand_num].id
participants_hash[participants[rand_num].id] += 1
  end
  # remove students who have already been assigned enough num of reviews out of participants array
  participants.each do |participant|
if participants_hash[participant.id] == student_review_num
  participants.delete_at(rand_num)
  num_participants -= 1
end
  end
end
  else
#review num for last team can be different from other teams.
#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
participants.each do |participant|
  # avoid last team receives too many peer reviews
  if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
selected_participants << participant.id
participants_hash[participant.id] += 1
  end
end
  end
 
  begin
selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
  rescue
flash[:error] = "Automatic assignment of reviewer failed."
  end
  iterator += 1
end
end
</pre>
</pre>
In addition to the classes generated Devise also generates a database migration in which fields related to the functionalities of these modules are added. Each field is related to a specific module and hence when a module is not require some of the fields may be removed from the migration to the database. Also most of these modules have specific forms and view associated with them. The forms are used by an end user to type in his/her information which will then be sent to the Devise controllers.


=='''Methods'''==
The method is made private so that it can only be called with in the controller and cannot directly be called through a view.
There are many classes in Devise which include models, controllers, helpers, views, routes etc. But much of the functionality offered by Devise is exposed via simple helper methods. Some of the most important methods which can be used in building our own application are:
 
<br/>
The complexity of the original method reduced after breaking it and can be easily readable now.
*'''authenticate_user! :''' This method is used to check whether a user is logged in before he/she attempts to perform a specific set of controller actions. authenticate_user! may be called with before_action as shown below to ensure the user is logged in before performing any of the operations.
 
before_action :authenticate_user!
If only some of the actions need authentication and some do not, we can use except clause so that only some actions are blocked as guest and others are accessible. The code with except clause is as below:
before_action :authenticate_user! except [:index, :show]
In the above example index and show are two controller actions associated with operations which do not require user authentication and can be browsed as a guest.Authenticate user may also be used with in a controller action so that it is application only for that specification. We need to use before_filter instead of before_action to achieve this purpose.
<pre>
<pre>
class EndUserBaseController < ApplicationController
def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
  before_filter :authentication_user!
    participants_hash = {}
end
    participants.each { |participant| participants_hash[participant.id] = 0 }
    #calculate reviewers for each team
    num_participants = participants.size
    if student_review_num != 0 and submission_review_num == 0
      num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
      exact_num_of_review_needed = participants.size * student_review_num
    elsif student_review_num == 0 and submission_review_num != 0
      num_reviews_per_team = submission_review_num
      student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
      exact_num_of_review_needed = teams.size * submission_review_num
    end
    #Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
    if student_review_num >= teams.size
      flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
    end
 
    peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
 
    # after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
    if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
      participants_with_insufficient_review_num = Array.new
      participants_hash.each do |participant_id, review_num|
        participants_with_insufficient_review_num << participant_id if review_num < student_review_num
      end
      unsorted_teams_hash = {}
      ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
        unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
          unsorted_teams_hash[response_map.reviewee_id] = 1
        else
          unsorted_teams_hash[response_map.reviewee_id] += 1
        end
      end
      teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
      participants_with_insufficient_review_num.each do |participant_id|
        teams_hash.each do |team_id, num_review_received|
          unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
            ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
            teams_hash[team_id] += 1
            teams_hash = teams_hash.sort_by{|_, v| v}.to_h
            break
          end
        end
      end
    end
    @@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
  end
</pre>
</pre>
In this example the application will authenticate a user only if he is trying to perform an action associated with the EndUserBaseController. If in any of the above cases a user is not logged in the application backs off and redirects to its sign-in page.
 
*'''current_user :''' current_user method is used to return the model class corresponding to the user who is currently signed in. For example, if you are building a messaging application, you may retrieve all the sent messages of a user as:
'''4. Moving code specific to models to models instead of controller'''
 
The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.
 
The following is the code snippet from the original method which contains the calls to ResponseMap model:
 
<pre>
<pre>
class SentMessagesController < ApplicationController
case @type
  before_filter :authentication_user!
    when "ReviewResponseMap"
  def index
      if params[:user].nil?
    @sent_messages = current_user.sent_messages.all
        # This is not a search, so find all reviewers for this assignment
  end
        response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
end
        @reviewers = []
        response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
          @reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
        end
        @reviewers = Participant.sort_by_name(@reviewers)
      else
        # This is a search, so find reviewers by user's full name
        user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
        @reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
      end
      #  @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
      # @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
      @review_scores = @assignment.compute_reviews_hash
      @avg_and_ranges= @assignment.compute_avg_and_ranges_hash
    when "FeedbackResponseMap"
      #Example query
      #SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and
      #reviewed_object_id in (select id from responses where
      #map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
      @review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
      @response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
      @reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
    when "TeammateReviewResponseMap"
      #Example query
      #SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
      @reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
    end
  end
</pre>
</pre>
Notice how authenticate_user! is used before checking the messages of the current user so as to ensure that the user is signed in before checking his/her messages.
 
*'''user_signed_in? :''' As the name suggests, user_signed_in? method is used to check whether a user is signed in. This is useful when you want to show two different pages depending on whether a user has logged in or not. For example, when a user is logged in you want to show him/her an option to Logout otherwise you want to show Register or Login options.
The same code after moving the methods to their respective models looks as follows:
 
<pre>
<pre>
<% if user_signed_in? %>
case @type
  <li><%- link_to "Logout", destroy_user_session_path, method :delete %></li>
      when "ReviewResponseMap"
<% else %>
        @review_user= params[:user]
  <li><%- link_to "Sign Up", new_user_registration_path %></li>
        #If review response is required call review_response_report method in review_response_map model
  <li><%- link_to "Login", new_user_session_path %></li>
        @reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
<% end %>
        @review_scores = @assignment.compute_reviews_hash
        @avg_and_ranges= @assignment.compute_avg_and_ranges_hash
      when "FeedbackResponseMap"
        #If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
        @reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
      when "TeammateReviewResponseMap"
        #If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
        @reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
    end
  end
</pre>
</pre>
We need to use method :delete to logout so that Devise will only logout when a HTTP delete request is made by user and does not accidentally logout because of a malicious link that automatically logs out the user.
=='''Testing UI'''==
*'''sign_in(@user) and sign_out(@user) :''' These methods are used to login(sign_in(@user)) or logout(sign_out(@user)) a newly created or existing user.
 
*'''user_session :''' This method returns metadata about the user that is currently logged in.
'''Peer review information:'''
The methods that are most frequently used by developers are current_user and user_signed_in? which are present as helper methods. Also if the methods are to be referred to an Admin then replace user in each method with admin i.e. current_user becomes current_admin etc.
* Instructor Login:
        Username: Instructor6
        Password: password
* Student login:
        Username: Student6384
        Password: password
 
'''Steps for testing UI:'''
# Login as an instructor (Using Instructor6 will help you import the participants from other assignments).
# Navigate to "Manage->Assignments".
# Click on "New Public Assignment" for creating a new assignment.
# Create a new assignment by providing assignment name, selecting a course, submission directory (Give any name) and description URL.
# Select "has teams?" and provide the team size. Click on create to create a new assignment.
## After that, click on review strategy and limit the number of reviews per submission.
## Click on "Due dates" and update date for submission and review. Adjust the review date and time in order to test the reviews.
## Click on "save". A new assignment is created and it can be viewed in "Manage->Assignments" section.
# In order to add participants, there are two methods to add students to the assignment.
## Click on "add participants" against the assignment. Enter the user login to add the student. Add atleast 6 participants so that the review mapping can be seen.
## Click on any previous assignment "add participants" and export the students list (I used wiki assignment).
## Click on your assignment "add participants" and import the students using the export file.
# Go back to "Assignments" section and click against "create teams".
## After clicking on "create teams", Click on "create teams" in the directed page.
### Teams can be formed either manually or automatically.
# Login as a student to submit the links for assignment (Valid hyperlink must be provided in the submission link). Add submission for atleast 5 students in order to check the automatic review mapping. (Password for student is password)
# After submitting the links for some students, Log in as an instructor to change the assignment phase to review phase.
## To change the review phase period, Go to assignments section and click on edit. Click on due dates and change the review due date.
# Now, login as a student and go to others work. Click on "Request a submission to review" and check whether a review is assigned automatically. If no assignments are submitted, then this cannot be tested.
# For teammate review report, author feedback report and review report, click against "review report" and all the review reports can be seen by selecting it in the drop down menu.
 
==References==


=='''See Also'''==
#[https://github.com/expertiza/expertiza Expertiza on GitHub]
=='''References'''==
#[https://github.com/harsha1007/expertiza Forked repository for the project]
#[http://expertiza.ncsu.edu/ Expertiza Main Page]

Latest revision as of 22:10, 1 April 2016

This page provides details about the OSS project which was based on refactoring one of controllers related to peer reviewing strategies used in Expertiza.


Expertiza

The Expertiza is a software project to create reusable learning objects through peer review. It is a Ruby on Rails based application which can be run on Windows, Linux and Mac OS X.

Main Features of Expertiza are:

  • Allows students to work in groups to improve others learning experiences
  • The work done by students/team is subjeted to multiple reviews , minimizing plagiarism
  • Reviewing is done by studnets, which allows instructor/TA to spend less time in grading
  • Large classes can also be handled easily with the help of Expertiza

About Review mapping controller

Review mapping controller contains methods related to peer reviewing strategies. It contains methods to add a reviewer, delete a reviewer, selecting a reviewer. Depending on the number of students and number of submissions, the topics to be reviewed are assigned to the students automatically. If a user wants to look for the team for a submission , it returns the team by comparing the submission id's with the team id's. Also, it assigns quizzes dynamically. Generation of review report, feedback report and teammate review is done.

Problem Statement

The main aim of this project is to

  • Refactor reporting method(response_report) which is big
  • Use more efficient function sample instead of shuffle for random number selection
  • Remove unused and assigned variables
  • Simplify Automatic_review_mapping_strategy

Code Improvements

Unused variables and arguments

There are unused variables in the methods which use the stack unnecessarily. So, it is better to remove the unused variables or at the least indicate that a variable is unused.

For suppose when both keys and values are not used in a hash but are given as arguments, then the unused variables can be indicated by adding a "_" infront of the name or replace the unused variable with "_" to represent it as unused variable but allow them in arguments.

Previous Code: teams_hash = unsorted_teams_hash.sort_by{|k, v| v}.to_h
After Changing the code: teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h

In the above case teams_hash should consist of a hash with both keys and values but the sorting is done based on values. So the key is replaced with a "_" so that the user may deem it unused in the implementation of the process.

Use sample instead of shuffle

When sample is used, the elements in an array are chosen by using random and unique indices in the array so that the elements doesn't repeat in the array. This cannot be guaranteed in shuffle. Also by using shuffle[0] we are shuffling all the elements in the array and then picking the first element instead of picking a single element randomly which is more efficient. The following are the couple of places where shuffle[0] was used and is replaced by sample.

Previous Code:

assignment_team = assignment_teams.to_a.shuffle[0] rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.shuffle[0] rescue nil
After Changing the code:

assignment_team = assignment_teams.to_a.sample rescue nil

topic = assignment.candidate_topics_to_review(reviewer).to_a.sample rescue nil

Cyclomatic complexity of automatic_review_mapping_strategy method

The method automatic_review_mapping_strategy handles the number of reviews assigned to a individual and also the number of reviews that can be done with in a team. The method is very long and has many nested if statements due to which the complexity of the method is very high. Instead of a single method handling all the parts of the strategy, it is divided into several parts due to which the code is more readable and also the complexity of code is shared by each method.

The method first checks the number of participants that are in an assignment and the number of teams that are present. It then sets the values for the maximum number of reviews that are possible and also the minimum number that are required. Then it assigns the reviews to each of the teams randomly when a request for review is made by a participant. At last it checks if the participant has the minimum number of reviews required after allotting the reviews. If not, it assigns more reviews to valid teams so that the minimum requirement is met.

The part of the code that is moved out of automatic_review_mapping_strategy as peer_review_strategy:

private
	def peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)
		iterator = 0
		teams.each do |team|
		  selected_participants = Array.new
		  if !team.equal? teams.last
			#need to even out the # of reviews for teams
			while selected_participants.size < num_reviews_per_team
			  num_participants_this_team = TeamsUser.where(team_id: team.id).size
			  #If there are some submitters or reviewers in this team, they are not treated as normal participants.
			  #They should be removed from 'num_participants_this_team'
			  TeamsUser.where(team_id: team.id).each do |team_user|
				temp_participant = Participant.where(user_id: team_user.user_id, parent_id: assignment_id).first
				num_participants_this_team -= 1 if temp_participant.can_review == false or temp_participant.can_submit == false
			  end
			  #if all outstanding participants are already in selected_participants, just break the loop.
			  break if selected_participants.size == participants.size - num_participants_this_team

			  # generate random number
			  if iterator == 0
				rand_num = rand(0..num_participants-1)
			  else
				min_value = participants_hash.values.min
				#get the temp array including indices of participants, each participant has minimum review number in hash table.
				participants_with_min_assigned_reviews = Array.new
				participants.each do |participant|
				  participants_with_min_assigned_reviews << participants.index(participant) if participants_hash[participant.id] == min_value
				end
				#if participants_with_min_assigned_reviews is blank 
				if_condition_1 = participants_with_min_assigned_reviews.empty?
				#or only one element in participants_with_min_assigned_reviews, prohibit one student to review his/her own artifact
				if_condition_2 = (participants_with_min_assigned_reviews.size == 1 and TeamsUser.exists?(team_id: team.id, user_id: participants[participants_with_min_assigned_reviews[0]].user_id))
				if if_condition_1 or if_condition_2
				  #use original method to get random number
				  rand_num = rand(0..num_participants-1)
				else
				  #rand_num should be the position of this participant in original array
				  rand_num = participants_with_min_assigned_reviews[rand(0..participants_with_min_assigned_reviews.size-1)]
				end
			  end
			  # prohibit one student to review his/her own artifact
			  next if TeamsUser.exists?(team_id: team.id, user_id: participants[rand_num].user_id)

			  if_condition_1 = (participants_hash[participants[rand_num].id] < student_review_num)
			  if_condition_2 = (!selected_participants.include? participants[rand_num].id)
			  if if_condition_1 and if_condition_2
				# selected_participants cannot include duplicate num
				selected_participants << participants[rand_num].id
				participants_hash[participants[rand_num].id] += 1
			  end 
			  # remove students who have already been assigned enough num of reviews out of participants array
			  participants.each do |participant|
				if participants_hash[participant.id] == student_review_num
				  participants.delete_at(rand_num)
				  num_participants -= 1
				end
			  end
			end
		  else
			#review num for last team can be different from other teams.
			#prohibit one student to review his/her own artifact and selected_participants cannot include duplicate num
			participants.each do |participant| 
			  # avoid last team receives too many peer reviews
			  if !TeamsUser.exists?(team_id: team.id, user_id: participant.user_id) and selected_participants.size < num_reviews_per_team
				selected_participants << participant.id 
				participants_hash[participant.id] += 1
			  end
			end
		  end

		  begin
			selected_participants.each {|index| ReviewResponseMap.where(:reviewee_id => team.id, :reviewer_id => index, :reviewed_object_id => assignment_id).first_or_create}
		  rescue
			flash[:error] = "Automatic assignment of reviewer failed."
		  end
		  iterator += 1
		end
	end

The method is made private so that it can only be called with in the controller and cannot directly be called through a view.

The complexity of the original method reduced after breaking it and can be easily readable now.

def automatic_review_mapping_strategy(assignment_id, participants, teams, student_review_num=0, submission_review_num=0)
    participants_hash = {}
    participants.each { |participant| participants_hash[participant.id] = 0 }
    #calculate reviewers for each team
    num_participants = participants.size
    if student_review_num != 0 and submission_review_num == 0
      num_reviews_per_team = (participants.size * student_review_num * 1.0 / teams.size).round
      exact_num_of_review_needed = participants.size * student_review_num
    elsif student_review_num == 0 and submission_review_num != 0
      num_reviews_per_team = submission_review_num
      student_review_num = (teams.size * submission_review_num * 1.0 / participants.size).round
      exact_num_of_review_needed = teams.size * submission_review_num
    end
    #Exception detection: If instructor want to assign too many reviews done by each student, there will be an error msg.
    if student_review_num >= teams.size
      flash[:error] = 'You cannot set the number of reviews done by each student to be greater than or equal to total number of teams [or "participants" if it is an individual assignment].'
    end

    peer_review_strategy(teams, num_participants, student_review_num, participants, participants_hash)

    # after assigning peer reviews for each team, if there are still some peer reviewers not obtain enough peer review, just assign them to valid teams
    if ReviewResponseMap.where(["reviewed_object_id = ? and created_at > ? and calibrate_to = ?", assignment_id, @@time_create_last_review_mapping_record, 0]).size < exact_num_of_review_needed
      participants_with_insufficient_review_num = Array.new
      participants_hash.each do |participant_id, review_num|
        participants_with_insufficient_review_num << participant_id if review_num < student_review_num
      end
      unsorted_teams_hash = {}
      ReviewResponseMap.where(["reviewed_object_id = ? and calibrate_to = ?", assignment_id, 0]).each do |response_map|
        unless unsorted_teams_hash.has_key? (response_map.reviewee_id)
          unsorted_teams_hash[response_map.reviewee_id] = 1 
        else
          unsorted_teams_hash[response_map.reviewee_id] += 1
        end
      end 
      teams_hash = unsorted_teams_hash.sort_by{|_, v| v}.to_h
      participants_with_insufficient_review_num.each do |participant_id|
        teams_hash.each do |team_id, num_review_received|
          unless TeamsUser.exists?(team_id: team_id, user_id: Participant.find(participant_id).user_id)
            ReviewResponseMap.where(:reviewee_id => team_id, :reviewer_id => participant_id, :reviewed_object_id => assignment_id).first_or_create
            teams_hash[team_id] += 1
            teams_hash = teams_hash.sort_by{|_, v| v}.to_h
            break
          end
        end
      end
    end
    @@time_create_last_review_mapping_record = ReviewResponseMap.where(reviewed_object_id: assignment_id).last.created_at
  end

4. Moving code specific to models to models instead of controller

The method response_report contains code which generates reports based on the input of one of the three of 'ReviewResponseMap', 'FeedbackResponseMap', 'TeammateReviewResponseMap' and 'Calibration'. Calibration is a special case and should be handled in the controller itself. But the other three are models that are subclasses of ResponseMap model. The report is generated for each of the three by calling the ResponseMap model and obtaining the values by querying the database. Moving the code to the models and calling the methods in the model through the controller makes more sense than writing the code in controller.

The following is the code snippet from the original method which contains the calls to ResponseMap model:

case @type
    when "ReviewResponseMap"
      if params[:user].nil?
        # This is not a search, so find all reviewers for this assignment
        response_maps_with_distinct_participant_id = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ? and calibrate_to = ?", @id, @type, 0])
        @reviewers = []
        response_maps_with_distinct_participant_id.each do |reviewer_id_from_response_map|
          @reviewers << (AssignmentParticipant.find(reviewer_id_from_response_map.reviewer_id))
        end
        @reviewers = Participant.sort_by_name(@reviewers)
      else
        # This is a search, so find reviewers by user's full name
        user = User.select("DISTINCT id").where(["fullname LIKE ?", '%'+params[:user][:fullname]+'%'])
        @reviewers = AssignmentParticipant.where(["user_id IN (?) and parent_id = ?", user, @assignment.id])
      end
      #  @review_scores[reveiwer_id][reviewee_id] = score for assignments not using vary_rubric_by_rounds feature
      # @review_scores[reviewer_id][round][reviewee_id] = score for assignments using vary_rubric_by_rounds feature
      @review_scores = @assignment.compute_reviews_hash
      @avg_and_ranges= @assignment.compute_avg_and_ranges_hash
    when "FeedbackResponseMap"
      #Example query
      #SELECT distinct reviewer_id FROM response_maps where type = 'FeedbackResponseMap' and 
      #reviewed_object_id in (select id from responses where 
      #map_id in (select id from response_maps where reviewed_object_id = 722 and type = 'ReviewResponseMap'))
      @review_response_map_ids = ResponseMap.select("id").where(["reviewed_object_id = ? and type = ?", @id, 'ReviewResponseMap'])
      @response_ids = Response.select("id").where(["map_id IN (?)", @review_response_map_ids])
      @reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id IN (?) and type = ?", @response_ids, @type])
    when "TeammateReviewResponseMap"
      #Example query
      #SELECT distinct reviewer_id FROM response_maps where type = 'TeammateReviewResponseMap' and reviewed_object_id = 711
      @reviewers = ResponseMap.select("DISTINCT reviewer_id").where(["reviewed_object_id = ? and type = ?", @id, 'TeammateReviewResponseMap'])
    end
  end

The same code after moving the methods to their respective models looks as follows:

case @type
      when "ReviewResponseMap"
        @review_user= params[:user]
        #If review response is required call review_response_report method in review_response_map model
        @reviewers = ReviewResponseMap.review_response_report(@id, @assignment,@type, @review_user)
        @review_scores = @assignment.compute_reviews_hash
        @avg_and_ranges= @assignment.compute_avg_and_ranges_hash
      when "FeedbackResponseMap"
        #If review report for feedback is required call feedback_response_report method in feedback_review_response_map model
        @reviewers = FeedbackResponseMap.feedback_response_report(@id, @type)
      when "TeammateReviewResponseMap"
        #If review report for teammate is required call teammate_response_report method in teammate_review_response_map model
        @reviewers = TeammateReviewResponseMap.teammate_response_report(@id)
    end
  end

Testing UI

Peer review information:

  • Instructor Login:
       Username: Instructor6
       Password: password
  • Student login:
       Username: Student6384
       Password: password

Steps for testing UI:

  1. Login as an instructor (Using Instructor6 will help you import the participants from other assignments).
  2. Navigate to "Manage->Assignments".
  3. Click on "New Public Assignment" for creating a new assignment.
  4. Create a new assignment by providing assignment name, selecting a course, submission directory (Give any name) and description URL.
  5. Select "has teams?" and provide the team size. Click on create to create a new assignment.
    1. After that, click on review strategy and limit the number of reviews per submission.
    2. Click on "Due dates" and update date for submission and review. Adjust the review date and time in order to test the reviews.
    3. Click on "save". A new assignment is created and it can be viewed in "Manage->Assignments" section.
  6. In order to add participants, there are two methods to add students to the assignment.
    1. Click on "add participants" against the assignment. Enter the user login to add the student. Add atleast 6 participants so that the review mapping can be seen.
    2. Click on any previous assignment "add participants" and export the students list (I used wiki assignment).
    3. Click on your assignment "add participants" and import the students using the export file.
  7. Go back to "Assignments" section and click against "create teams".
    1. After clicking on "create teams", Click on "create teams" in the directed page.
      1. Teams can be formed either manually or automatically.
  8. Login as a student to submit the links for assignment (Valid hyperlink must be provided in the submission link). Add submission for atleast 5 students in order to check the automatic review mapping. (Password for student is password)
  9. After submitting the links for some students, Log in as an instructor to change the assignment phase to review phase.
    1. To change the review phase period, Go to assignments section and click on edit. Click on due dates and change the review due date.
  10. Now, login as a student and go to others work. Click on "Request a submission to review" and check whether a review is assigned automatically. If no assignments are submitted, then this cannot be tested.
  11. For teammate review report, author feedback report and review report, click against "review report" and all the review reports can be seen by selecting it in the drop down menu.

References

  1. Expertiza on GitHub
  2. Forked repository for the project
  3. Expertiza Main Page