CSC/ECE 517 Fall 2014/OSS E1466 gjf: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
m (Created page with "''' CANVAS 2D with RUST and MOZILLA SERVO ''' This wiki deals with our implementation of a<code> [http://en.wikipedia.org/wiki/Canvas_element CANVAS] </code> for the Mozilla Ser...")
 
 
(117 intermediate revisions by 3 users not shown)
Line 1: Line 1:
''' CANVAS 2D with RUST and MOZILLA SERVO '''
==E1466: Refactoring GradesController==
 
This wiki deals with our implementation of a controller in expertiza:
[https://github.com/expertiza/expertiza/blob/rails4/app/controllers/grades_controller.rb grade_controller]  for the Expertiza Project using Ruby on Rails.


This wiki deals with our implementation of a<code> [http://en.wikipedia.org/wiki/Canvas_element CANVAS] </code> for the Mozilla Servo Project using the Rust Programming Language.


== Introduction ==
== Introduction ==


Mozilla are working on a project called Servo
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.
 
==Project Description==
<br>
'''Classes involved:''' grades_controller.rb
 
'''What it does:'''
 
This class lists the grades of all the participants for an assignments and also their reviews. Instructor can edit scores, calculate penalties and send emails for conflicts.
 
'''What needs to be done:'''
 
*Modify '''calculate_all_penalties''' method which is too complex and long.
*Put the '''get_body_text''' method in a mailer rather than in the grades_controller.
*Refactor '''conflict_nofication''' method to '''conflict_email''' and make it delegated to the mailer.
*Refactor '''view_my_scores''' method to '''grades_show''' and delete the unnecessary variables.
*Try not to query for reviews and meta reviews in '''grades_show''' method.
 
'''What we have done:'''
*Modify '''calculate_all_penalties''' method.
*Refactor the '''conflict_notification''' method to '''conflict_email''' method.
*Simplify sending email function and make it delegated to the mailer.
*Send a conflict email to the reviewer automatically when instructor click the button "email reviewer".
*Remove '''get_body_text''' and '''send_grading_email''' method from grades_controller.
*Refactor '''conflict_nofication''' method to '''conflict_email''' and make it delegated to the mailer.
*Use Design Pattern Delegation in ConflictMailer.
*Refactor '''view_my_scores''' method to '''grades_show''' method.
*Search for the key reasons which lead to huge waiting time for getting score.
*Refactor '''get_assessments_for''' method in response_map.rb and lead to more than 90% off the current waiting time.
*Eliminate the search for reviews and meta reviews during the '''grades_show''' method.
*Delete unnecessary instance Variables.
 
==Set Up Environment==
 
===User Guide===
Current System (After Refactoring Grades_controller):
 
http://152.46.18.10:3000/
 
Instructor: user6 Password: test
 
Student: user1600 and user1601 Password: test
 
Original System (Before Refactoring Grades_controller):
 
http://152.1.13.97:3000/
 
Instructor: user6 Password: test
 
Student: user1600 and user1601 Password: test


== Design ==
All our test result based on the following test cases on expertiza, please follow these step to get it.


=== Rust ===
Instructor: (Searching "Program 2" using "Ctrl + F" will be convinient for you.)


<code>[http://en.wikipedia.org/wiki/Rust_(programming_language) Rust] </code> is a curly-brace, block-structured expression language.
Steps: Login -> Assignments->Program 2 style ->view scores.


It visually resembles the C language family, but differs significantly in syntactic and semantic details.
Student:


Its design is oriented toward concerns of “programming in the large”, that is, of creating and maintaining boundaries – both abstract and operational – that preserve large-system integrity, availability and concurrency.
  Steps: Login -> Assignments->Program 2 style ->Your scores.


It supports a mixture of imperative procedural, concurrent actor, object-oriented and pure functional styles. Rust also supports generic programming and metaprogramming, in both static and dynamic styles.  
===Set Up Environment Locally===
====Get Expertiza from Github====
<pre style="white-space:normal;">git clone https://github.com/maxlpy/expertiza.git</pre>
====Use rvm to install ruby-2.1.0====
<pre style="white-space:normal;">curl -L https://get.rvm.io | bash -s stable</pre>
<pre style="white-space:normal;">source ~/.rvm/scripts/rvm</pre>
<pre style="white-space:normal;">echo "source ~/.rvm/scripts/rvm" >> ~/.bashrc</pre>
<pre style="white-space:normal;">rvm install 2.1.0</pre>
<pre style="white-space:normal;">rvm use 2.1.0 --default</pre>
<pre style="white-space:normal;">ruby -v</pre>


Example:
====Install Bundled Gems====
                  fn main() {
Set JAVA_HOME for the rjb gem:
  println("hello, world");
Your path may be different. You can generally find out the path by looking at the symbolic link at /etc/alternatives/java
                        }
<pre style="white-space:normal;">ls -la /etc/alternatives/java</pre>
This outputs something like '/usr/lib/jvm/java-6-openjdk-amd64/jre/bin/java'. Only part of this path may need to be set to JAVA_HOME. In this instance, it is '/usr/lib/jvm/java-6-openjdk-amd64'.
<pre style="white-space:normal;">export JAVA_HOME=/usr/lib/jvm/java-6-openjdk-amd64</pre>
<pre style="white-space:normal;">bundle install</pre>


=== Servo ===  
====Set Up the Database====
=====Enable and Start the MySql Daemon=====
<pre style="white-space:normal;">sudo service mysqld enable</pre>
<pre style="white-space:normal;">sudo service mysqld start</pre>
=====Set the MySql Root Password=====
<pre style="white-space:normal;">mysqladmin -u root password</pre>
=====Log in to MySql=====
<pre style="white-space:normal;">mysql -uroot -p</pre>
====Create the Databases====
<pre style="white-space:normal;">rake db:create:all</pre>
====Build the Expertiza Database====
<pre style="white-space:normal;">rake db:migrate</pre>
====Start Expertiza service====
<pre style="white-space:normal;">rails server</pre>


<code>[http://en.wikipedia.org/wiki/Servo_(layout_engine) Servo ] </code> is an experimental web browser layout engine being developed by Mozilla.
==Code Modification==


The prototype seeks to create a highly parallel environment, in which many components (such as rendering, layout, HTML parsing, image decoding, etc.) are handled by fine-grained, isolated tasks.


The project has a symbiotic relationship with the Rust programming language, in which it is being developed.


Servo is explicitly not aiming to create a full Web browser (except for demonstration and experimentation purposes). Rather it is focused on creating a solid, embeddable engine. Although Servo is a research project, it is designed to be "productizable"—the code that we write should be of high enough quality that it could eventually be shipped to users.
=== calculate_all_penalties Method===
====Summary====


==== Design Diagrams ====
We have modified the calculate_penalties method and implemented all the functions it originally has. After our modification the method shrinks from 45 lines to 32 lines of Ruby code. Originally, the method includes a lot of unnecessary if and unless statements which makes the method uneasy to read and seems to have a difficult logic to calculate the penalties.


'''Task Supervision Diagram'''
According to the [https://codeclimate.com codeclimate] which calculate the complexity of the method, originally, it has a complexity of '''85''', however, after we modified it, the complexity goes down to '''65'''. Please check at [https://codeclimate.com/github/expertiza/expertiza/GradesController original version],[https://codeclimate.com/github/maxlpy/expertiza/GradesController modified version]


[[File:Diagram1.jpg]]
We have done the following to the original methods:


'''Task Communication Diagram'''
*Used for loop with selective choice to create penalty attributes and put it into the '''CalculatedPenalty''' Table, instead of duplicating the almost same code.


[[File:Diagram2.jpg]]
*Used Object-Oriented Design patterns of Ruby: let the array respond to the method '''min''' which returns the minimum number in the array and get the logical penalty other than using if statement.(max_penalty if total_penalty is larger than max_penalty,  total_penalty if total_penalty is smaller than max_penalty)


• ''Each box'' represents a Rust task.
*Eliminated the unnecessary if statements to make the code clear to read.


• ''Blue boxes'' represent the primary tasks in the browser pipeline.
====Implementation====


• ''Gray boxes'' represent tasks auxiliary to the browser pipeline.
*Modify if statements:


• ''White boxes'' represent worker tasks. Each such box represents several tasks, the precise number of which will vary with the workload.
Before Refactoring:
<pre>
if(penalties[:submission].nil?)
      penalties[:submission]=0
end
if(penalties[:review].nil?)
      penalties[:review]=0
end
if(penalties[:meta_review].nil?)
      penalties[:meta_review]=0
end
</pre>
After Refactoring:
<pre>
penalties[:submission] = 0 if penalties[:submission].nil?
penalties[:review] = 0 if penalties[:review].nil?
penalties[:meta_review] = 0 if penalties[:meta_review].nil?
</pre>


• ''Dashed lines'' indicate supervisor relationships.
*Using Objected-oriented Pattern
Before Refactoring:
<pre>
if(@total_penalty > l_policy.max_penalty)
      @total_penalty = l_policy.max_penalty
end
</pre>
After Refactoring:
<pre>
@total_penality=[l_policy.max_penalty,@total_penality].min
</pre>
*Using loops rather than duplicate same code
Before Refactoring:
<pre>
penalty_attr1 = {:deadline_type_id => 1,:participant_id => @participant.id, :penalty_points => penalties[:submission]}
      CalculatedPenalty.create(penalty_attr1)
penalty_attr2 = {:deadline_type_id => 2,:participant_id => @participant.id, :penalty_points => penalties[:review]}
      CalculatedPenalty.create(penalty_attr2)
penalty_attr3 = {:deadline_type_id => 5,:participant_id => @participant.id, :penalty_points => penalties[:meta_review]}
      CalculatedPenalty.create(penalty_attr3)"
@all_penalties[participant.id] = Hash.new
      @all_penalties[participant.id][:submission] = penalties[:submission]
      @all_penalties[participant.id][:review] = penalties[:review]
      @all_penalties[participant.id][:meta_review] = penalties[:meta_review]
      @all_penalties[participant.id][:total_penalty] = @total_penalty"
</pre>
After Refactoring:
<pre>
deadline_type=[1,2,5]
penalty_type=[:submission,:review,:meta_review]
if calculate_for_participants
      for i in 0..2
          penalty_attr={:deadline_type_id=>deadline_type[i],:participant_id => @participant.id, :penalty_points => penalties[penalty_type[i]]}
          CalculatedPenalty.create(penalty_attr)
      end
end
@all_penalties[participant.id] = {}
      for i in 0..2
          @all_penalties[participant.id][:penalty_type[i]] = penalties[:penalty_type[i]]
      end
@all_penalties[participant.id][:total_penalty] = @total_penalty"
</pre>


• ''Solid lines'' indicate communication channels.
=== conflict_notification Method===


==Project Description==
====Summary====
<br>
We refactored the '''conflict_notification''' method to '''conflict_email''' method. When instructor click the button "email reviewer" whom he thought give a unfair review, the system should be able to send an email to the reviewer automatically.The '''get_body_text''' and '''send_grading_email''' were removed from the controller. The sending email function is well simplified and delegated to a conflict_mailer.
The project requirement stated us to develop a ''Binding'' for a '''CanvasRenderingContext2D.webidl''' and the '''HTMLCanvasElement.webidl'''  interface.
 
====Design Pattern====
Delegation: The conflict_email should be able to send email. The class ConflictMailer works as its delegate .The conflict_email calls the send_conflict_email function to take the job of sending an email.
 
UML: Draw UML graph for Grades_controller
 
====Implementation====
The old '''conflict_notification''' action queries an email address list of a certain kind of reviews. In its view conflict_notifiction.html.erb, A email form contains email list for instructor to choose from and email content.
[[File:old_email.png]]
 
'''Before Refactorring''', in conflict_notification:
First,Get the instructor email address as sender
<pre>
      if session[:user].role_id !=6           
        @instructor = session[:user]
      else
        @instructor = Ta.get_my_instructor(session[:user].id)
      end
      @participant = AssignmentParticipant.find(params[:id])
      @assignment = Assignment.find(@participant.parent_id)
</pre>
Then,Based on submission of review, the action query all the reviewers' email address 
<pre> 
      @questions = Hash.new
      questionnaires = @assignment.questionnaires
      questionnaires.each {
        |questionnaire|
        @questions[questionnaire.symbol] = questionnaire.questions
      }
</pre>
The email list is for instructor to choose from. The process_response method queries the email address.   
<pre> 
      @reviewers_email_hash = Hash.new           
      @caction = "view"
      @submission = params[:submission]
      if @submission == "review"
        @caction = "view_review"
        @symbol = "review"
        process_response("Review", "Reviewer", @participant.reviews, "ReviewQuestionnaire")
      elsif @submission == "review_of_review"
        @symbol = "metareview"
        process_response("Metareview", "Metareviewer", @participant.metareviews, "MetareviewQuestionnaire")
      elsif @submission == "review_feedback"
        @symbol = "feedback"
        process_response("Feedback", "Author", @participant.feedback, "AuthorFeedbackQuestionnaire")
      elsif @submission == "teammate_review"
        @symbol = "teammate"
        process_response("Teammate Review", "Reviewer", @participant.teammate_reviews, "TeammateReviewQuestionnaire")
      end
</pre>
The get_body_text method construct email content which should be removed from controller.
<pre>                                             
      @body = get_body_text(params[:submission])     
</pre>
 
The conflict_notification action requiring another two method "process_response" and "get_body_text" method to get the recipient email list and email content which contributes to extra complexity. The instructor only have to click the button "send email" in conflict_notifiction.html.erb and turn to send_conflict_email action, then he/she can send the email. If the instructor want to send another email, the process will repeat again,which will be terrible when reloading the "view" scores page.
 
'''After Refactoring''':The querying reviews actually have done in our show_reviews action, and we can get the individual reviewers' email address.In the reviewer table,we add extra button "email reviewer".Now the instructor could send to reviewer email directly.
[[File:conflict_email.png]]
 
 
In _review_table.html.erb,we add the link to send email. The review_id is passed to the conflict_email action taken as the recipient.
<pre>
<%= link_to 'email reviewer', :action => 'conflict_email',:remote=> true,:reviewer_id=>
 
review.map.reviewer.user,:participant_id=>participant,:score=>sprintf("%.2f",score) %>
</pre>
 
In our new '''conflict_email''' action:
 
We still need the instructor as the sender
<pre>
      if session[:user].role_id !=6
        @instructor = session[:user]
      else
        @instructor = Ta.get_my_instructor(session[:user].id)
      end
</pre>
The reviewer_id is passed from the link,we can query his/her email address to send the email
<pre>
      recipient=User.find(params[:reviewer_id])
</pre>
The responsibility of sending email has been delegated to a new mailer conflict_mailer. And the email content was constructed in the mailer. 
<pre>
      participant = AssignmentParticipant.find(params[:participant_id])
      assignment=Assignment.find(participant.parent_id)     
      score=params[:score]
      ConflictMailer.send_conflict_email(instructor,recipient,assignment,score).deliver
 
</pre>
Take a look at new conflict_mailer, it define a '''send_conflict_email''' method. the email was sent from sender's email and to reviewer's email.Some instance variables were used in view.
In conflict_mailer.erb
<pre>
  def send_conflict_email(sender,recipient,assignment,score)
      @assignment=assignment
      @recipient=recipient
      @score=score
      if recipient.role_id==1
        @role = "reviewer"
        @item = "submission"
      else
        @role = "metareviewer"
        @item = "review"
      end
    mail subject: "Confliction Email",from: sender.email,to: reviewer.email
  end
</pre>
In its view send_conflict_email.txt.haml, we edit the email content.By this way, we deleted the '''get_body_text''' and well followed the rule of MVC design
<pre>
%h1="Hi ," +@recipient.name
 
%p="You submitted a score of "+@score+" for assignment "+@assignment.name+" that varied greatly from another "+@role+"'s score for the same "+@item+"."


• We had to write a '''canvasrenderingcontext2d.rs''' file, which is written in the Rust programming language. We needed a struct containing a Reflector at minimum, as well as implementation of the Reflectable and BindingObject traits.
%p="The Expertiza system has brought this to my attention."
</pre>


• We also had to write an '''htmlcanvaselement.rs''' file which is also written in the Rust Language.


The above files get compiled to generate ''Binding Files'' named '''CanvasRenderingContext2DBinding.rs''' and '''HTMLCanvasELementBinding.rs'''
The link "email reviewer" request a ajax,so the current view page will not refresh when the action done
<pre>
      respond_to do |format|
        format.js
      end
</pre>
If an email was successfully sent,The instructor should get the alert window by ajax.
[[File:email_sent.png]]


• We then added '''<canvas>''' element support to the ''HTML parser'' using the '''hubbub_html_parser.rs''' file.
=== view_my_scores method===


• Then we successfully built and compiled our project on the Servo Engine.
====Summary====
We refactored '''view_my_scores''' method to grades_show, deleted many unnecessary instance variables along with several review and meta reviews methods inside it. Therefore, the system will only search for scores in the database rather than search for scores and reviews, which wastes a lot of time during the grade show process. After refactoring, the complexity on this decreased from '''53''' to '''35'''.
We also optimize '''get_scores''' method to improve the efficiency of showing scores by students and instructors, which cost us most of the time during the project. After refactoring this method, the time cost of views in grade controller decreased by more than '''90%'''.


• This was Part one of our Mozilla project.
We have done the following to the original methods:


• Our Final Project of OOLS will consist of Part two of the Mozilla project which includes implementing '''Canvas API''' for Mozilla using the '''CanvasRenderingContext2D''' and the '''HTMLCanvasElement''' interfaces.
*Refactored '''view_my_scores''' method to '''grades_show''' method.


===Screenshots===
*Searched for the key reasons which lead to huge waiting time for getting score.


*Refactored '''get_assessments_for''' method in response_map.rb and lead to more than 90% off the current waiting time.


'''Make and Build Servo'''
*Eliminated the search for reviews during the '''grades_show''' method.


[[File:Sc1.jpg]]
*Deleted unnecessary instance Variables.


====Implementation====
*Deleted the unnecessary instance variables


Deleted Code:
<pre>
@average_score_results = Array.new
@average_score_results = ScoreCache.get_class_scores(@participant.id)
@statistics = Array.new
@average_score_results.each { |x|
@statistics << x
}
</pre>


'''Make Check Successful'''
*Deleted query for reviews and meta_reviews


[[File:Sc2.jpg]]
Deleted Code:
<pre>
@average_reviews = ScoreCache.get_reviews_average(@participant.id)
@average_metareviews = ScoreCache.get_metareviews_average(@participant.id)
@my_reviews = ScoreCache.my_reviews(@participant.id)
@my_metareviews = ScoreCache.my_metareviews(@participant.id)
</pre>


*Modify '''get_assessments_for''' method in response_map.rb
After doing this, the time cost of view function decreased by more than 90%
Before Refactoring:
<pre>
# the original method to find all response
@all_resp=Response.all
for element in @all_resp
    if (element.map_id == map.map_id)
        @array_sort << element
        @test << map
    end
end
</pre>


After Refactoring:
<pre>
@all_resp=Response.find_by_map_id(map.map_id)
@array_sort << @all_resp
@test << map
</pre>


'''Run Servo Engine'''
*Add a new method into grades_controller.rb


[[File:Sc3.png]]
Add Method:
<pre>
#show_reviews method gets reviews, meta reviews and author feedback information from server.
  def show_reviews
    @partial='grades/'+params[:path]
    @prefix=params[:prefix]
    @score=Hash.new
    @score[:assessments]=Array.new
    params[:score][:assessments].each do |assessment|
        @score[:assessments]<<Response.find(assessment)
    end
    @score[:scores]=params[:score][:scores]
    @participant=AssignmentParticipant.find(params[:participant])
    @assignment = @participant.assignment
    @questions = {}
    questionnaires = @assignment.questionnaires
    questionnaires.each do |questionnaire|
      @questions[questionnaire.symbol] = questionnaire.questions
    end
  end
</pre>


==Future Scope==
*Change view_my_score.html.erb into show_reviews.html.erb in views.
<br>
Servo is predicted to be – the next Firefox


Note that Servo is a research project, it may or may not become an actual product. Firefox is evolved in parallel and making great progress. In other words, Mozilla does not make the mistake of betting the farm on Servo. A complete rewrite of parts of the Navigator web browser (the flagship product) for version 4 brought Mozilla’s ancestor Netscape much trouble.
*Add in views(show_reviews.html.erb)


===Goals===
<pre>
• The Servo project is about building a parallel browser:
<%= render :partial=>@partial, :locals => {:prefix => @prefix, :participant => @participant, :rscore => @score} %>
</pre>


• Use Rust instead of C++ which is safer and better equipped for parallelism.
====Test Result====
*The following test results based on the test case on Expertiza(Assignments: Program 2 style)
{| class="wikitable" style="font-size: 90%;"
|+ style="font-size: 1.25em;" |
|-
!style="background:#eFeFeF;"| Name
!style="background:#eFeFeF;"| Before Refactoring
!style="background:#eFeFeF;"| After Refactoring
!style="background:#eFeFeF;"| Reduced By


• Try to parallelize as many stages of the rendering process as possible (ongoing research).
|-
| View all team's score(instructor)
| 484988ms
| 8642ms
| 98.21%
|-
| View own score(student)
| 8941ms
| 651ms
| 92.71%
|}


• Implement the <code> [https://wiki.mozilla.org/DOM:Home_Page DOM] </code> mostly in JavaScript (which makes it faster, because there are less context switches).
*Original Time for Instructor to View all scores


• Browser innovation only: For the foreseeable future, Servo will use Firefox’s JavaScript engine, <code> [http://en.wikipedia.org/wiki/SpiderMonkey_(JavaScript_engine) Spidermonkey.] </code>
[[File:Instructor_old.png‎|frame|center|Original Time for Instructor to View all scores]]


==Appendix==
*Time for Instructor to View all scores after Refactoring


Steps to run the project
[[File:Instructor_new.png‎|frame|center|Time for Instructor to View all scores after Refactoring]]


1. Install and Build Rust (follow the link) -  [https://github.com/mozilla/rust/wiki/Note-getting-started-developing-Rust]
*Original Time for Student to View all scores


2. Build Servo            (follow the link) -  [https://github.com/mozilla/servo]
[[File:Student_old.png‎|frame|center|Original Time for Student to View all scores]]


3. Write the code for canvasrenderingcontext2d.rs and htmlcanvaselement.rs
*Time for Student to View all scores after Refactoring


4. Generate a Binding for CanvasRenderingContext2D.webidl, HTMLCanvasElement.webidl
[[File:Student_new.png‎|frame|center|Time for Student to View all scores after Refactoring]]


5. Run Servo


==References==
==See Also==
*[https://github.com/expertiza/expertiza Expertiza Website]


1. https://github.com/mozilla/rust/wiki/Note-getting-started-developing-Rust -Getting started with Rust
*[https://github.com/maxlpy/expertiza/blob/rails4/README.rdoc GradesController Readme File]


2. https://github.com/mozilla/servo - Getting started with servo
*[https://www.youtube.com/watch?v=Md4NO6VjCn8&feature=youtu.be GradesController Video Instructions]


3. https://github.com/mozilla/servo/wiki/Adding-a-new-WebIDL-binding - Adding a new webidl binding.
*[http://152.46.18.10:3000/ Original Expertiza Website]


4. https://github.com/Aalhad/CanvasRenderingContext2DMozilla - Our Project on github.
*[http://152.1.13.97:3000/ Expertiza Website After Refactoring]


5. http://www.2ality.com/2012/02/servo.html - Future Scope
*[https://codeclimate.com/github/expertiza/expertiza/GradesController Original GradesController on CodeClimate]


6.      http://en.wikipedia.org/wiki/Rust_(programming_language) - Rust
*[https://codeclimate.com/github/maxlpy/expertiza/GradesController  Modified GradesController on CodeClimate]

Latest revision as of 15:46, 1 December 2014

E1466: Refactoring GradesController

This wiki deals with our implementation of a controller in expertiza: grade_controller for the Expertiza Project using Ruby on Rails.


Introduction

Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

Project Description


Classes involved: grades_controller.rb

What it does:

This class lists the grades of all the participants for an assignments and also their reviews. Instructor can edit scores, calculate penalties and send emails for conflicts.

What needs to be done:

  • Modify calculate_all_penalties method which is too complex and long.
  • Put the get_body_text method in a mailer rather than in the grades_controller.
  • Refactor conflict_nofication method to conflict_email and make it delegated to the mailer.
  • Refactor view_my_scores method to grades_show and delete the unnecessary variables.
  • Try not to query for reviews and meta reviews in grades_show method.

What we have done:

  • Modify calculate_all_penalties method.
  • Refactor the conflict_notification method to conflict_email method.
  • Simplify sending email function and make it delegated to the mailer.
  • Send a conflict email to the reviewer automatically when instructor click the button "email reviewer".
  • Remove get_body_text and send_grading_email method from grades_controller.
  • Refactor conflict_nofication method to conflict_email and make it delegated to the mailer.
  • Use Design Pattern Delegation in ConflictMailer.
  • Refactor view_my_scores method to grades_show method.
  • Search for the key reasons which lead to huge waiting time for getting score.
  • Refactor get_assessments_for method in response_map.rb and lead to more than 90% off the current waiting time.
  • Eliminate the search for reviews and meta reviews during the grades_show method.
  • Delete unnecessary instance Variables.

Set Up Environment

User Guide

Current System (After Refactoring Grades_controller):

http://152.46.18.10:3000/

Instructor: user6 Password: test

Student: user1600 and user1601 Password: test

Original System (Before Refactoring Grades_controller):

http://152.1.13.97:3000/

Instructor: user6 Password: test

Student: user1600 and user1601 Password: test

All our test result based on the following test cases on expertiza, please follow these step to get it.

Instructor: (Searching "Program 2" using "Ctrl + F" will be convinient for you.)

Steps: Login -> Assignments->Program 2 style ->view scores.

Student:

Steps: Login -> Assignments->Program 2 style ->Your scores.

Set Up Environment Locally

Get Expertiza from Github

git clone https://github.com/maxlpy/expertiza.git

Use rvm to install ruby-2.1.0

curl -L https://get.rvm.io | bash -s stable
source ~/.rvm/scripts/rvm
echo "source ~/.rvm/scripts/rvm" >> ~/.bashrc
rvm install 2.1.0
rvm use 2.1.0 --default
ruby -v

Install Bundled Gems

Set JAVA_HOME for the rjb gem: Your path may be different. You can generally find out the path by looking at the symbolic link at /etc/alternatives/java

ls -la /etc/alternatives/java

This outputs something like '/usr/lib/jvm/java-6-openjdk-amd64/jre/bin/java'. Only part of this path may need to be set to JAVA_HOME. In this instance, it is '/usr/lib/jvm/java-6-openjdk-amd64'.

export JAVA_HOME=/usr/lib/jvm/java-6-openjdk-amd64
bundle install

Set Up the Database

Enable and Start the MySql Daemon
sudo service mysqld enable
sudo service mysqld start
Set the MySql Root Password
mysqladmin -u root password
Log in to MySql
mysql -uroot -p

Create the Databases

rake db:create:all

Build the Expertiza Database

rake db:migrate

Start Expertiza service

rails server

Code Modification

calculate_all_penalties Method

Summary

We have modified the calculate_penalties method and implemented all the functions it originally has. After our modification the method shrinks from 45 lines to 32 lines of Ruby code. Originally, the method includes a lot of unnecessary if and unless statements which makes the method uneasy to read and seems to have a difficult logic to calculate the penalties.

According to the codeclimate which calculate the complexity of the method, originally, it has a complexity of 85, however, after we modified it, the complexity goes down to 65. Please check at original version,modified version

We have done the following to the original methods:

  • Used for loop with selective choice to create penalty attributes and put it into the CalculatedPenalty Table, instead of duplicating the almost same code.
  • Used Object-Oriented Design patterns of Ruby: let the array respond to the method min which returns the minimum number in the array and get the logical penalty other than using if statement.(max_penalty if total_penalty is larger than max_penalty, total_penalty if total_penalty is smaller than max_penalty)
  • Eliminated the unnecessary if statements to make the code clear to read.

Implementation

  • Modify if statements:

Before Refactoring:

 if(penalties[:submission].nil?)
      penalties[:submission]=0
 end
 if(penalties[:review].nil?)
      penalties[:review]=0
 end
 if(penalties[:meta_review].nil?)
      penalties[:meta_review]=0
 end

After Refactoring:

penalties[:submission] = 0 if penalties[:submission].nil?
penalties[:review] = 0 if penalties[:review].nil?
penalties[:meta_review] = 0 if penalties[:meta_review].nil?
  • Using Objected-oriented Pattern

Before Refactoring:

 if(@total_penalty > l_policy.max_penalty)
      @total_penalty = l_policy.max_penalty
 end

After Refactoring:

 @total_penality=[l_policy.max_penalty,@total_penality].min
  • Using loops rather than duplicate same code

Before Refactoring:

 penalty_attr1 = {:deadline_type_id => 1,:participant_id => @participant.id, :penalty_points => penalties[:submission]}
       CalculatedPenalty.create(penalty_attr1)
 penalty_attr2 = {:deadline_type_id => 2,:participant_id => @participant.id, :penalty_points => penalties[:review]}
       CalculatedPenalty.create(penalty_attr2)
 penalty_attr3 = {:deadline_type_id => 5,:participant_id => @participant.id, :penalty_points => penalties[:meta_review]}
       CalculatedPenalty.create(penalty_attr3)"
 @all_penalties[participant.id] = Hash.new
       @all_penalties[participant.id][:submission] = penalties[:submission]
       @all_penalties[participant.id][:review] = penalties[:review]
       @all_penalties[participant.id][:meta_review] = penalties[:meta_review]
       @all_penalties[participant.id][:total_penalty] = @total_penalty"

After Refactoring:

 deadline_type=[1,2,5]
 penalty_type=[:submission,:review,:meta_review]
 if calculate_for_participants
      for i in 0..2
           penalty_attr={:deadline_type_id=>deadline_type[i],:participant_id => @participant.id, :penalty_points => penalties[penalty_type[i]]}
           CalculatedPenalty.create(penalty_attr)
      end
 end
 @all_penalties[participant.id] = {}
      for i in 0..2
          @all_penalties[participant.id][:penalty_type[i]] = penalties[:penalty_type[i]]
      end
 @all_penalties[participant.id][:total_penalty] = @total_penalty"

conflict_notification Method

Summary

We refactored the conflict_notification method to conflict_email method. When instructor click the button "email reviewer" whom he thought give a unfair review, the system should be able to send an email to the reviewer automatically.The get_body_text and send_grading_email were removed from the controller. The sending email function is well simplified and delegated to a conflict_mailer.

Design Pattern

Delegation: The conflict_email should be able to send email. The class ConflictMailer works as its delegate .The conflict_email calls the send_conflict_email function to take the job of sending an email.

UML: Draw UML graph for Grades_controller

Implementation

The old conflict_notification action queries an email address list of a certain kind of reviews. In its view conflict_notifiction.html.erb, A email form contains email list for instructor to choose from and email content.

Before Refactorring, in conflict_notification: First,Get the instructor email address as sender

 
      if session[:user].role_id !=6             
        @instructor = session[:user]
      else
        @instructor = Ta.get_my_instructor(session[:user].id)
      end
      @participant = AssignmentParticipant.find(params[:id])
      @assignment = Assignment.find(@participant.parent_id)

Then,Based on submission of review, the action query all the reviewers' email address

  
      @questions = Hash.new
      questionnaires = @assignment.questionnaires
      questionnaires.each {
        |questionnaire|
        @questions[questionnaire.symbol] = questionnaire.questions
      }

The email list is for instructor to choose from. The process_response method queries the email address.

  
      @reviewers_email_hash = Hash.new            
      @caction = "view"
      @submission = params[:submission]
      if @submission == "review"
        @caction = "view_review"
        @symbol = "review"
        process_response("Review", "Reviewer", @participant.reviews, "ReviewQuestionnaire")
      elsif @submission == "review_of_review"
        @symbol = "metareview"
        process_response("Metareview", "Metareviewer", @participant.metareviews, "MetareviewQuestionnaire")
      elsif @submission == "review_feedback"
        @symbol = "feedback"
        process_response("Feedback", "Author", @participant.feedback, "AuthorFeedbackQuestionnaire")
      elsif @submission == "teammate_review"
        @symbol = "teammate"
        process_response("Teammate Review", "Reviewer", @participant.teammate_reviews, "TeammateReviewQuestionnaire")
      end

The get_body_text method construct email content which should be removed from controller.

                                               
      @body = get_body_text(params[:submission])      

The conflict_notification action requiring another two method "process_response" and "get_body_text" method to get the recipient email list and email content which contributes to extra complexity. The instructor only have to click the button "send email" in conflict_notifiction.html.erb and turn to send_conflict_email action, then he/she can send the email. If the instructor want to send another email, the process will repeat again,which will be terrible when reloading the "view" scores page.

After Refactoring:The querying reviews actually have done in our show_reviews action, and we can get the individual reviewers' email address.In the reviewer table,we add extra button "email reviewer".Now the instructor could send to reviewer email directly.


In _review_table.html.erb,we add the link to send email. The review_id is passed to the conflict_email action taken as the recipient.

<%= link_to 'email reviewer', :action => 'conflict_email',:remote=> true,:reviewer_id=> 

review.map.reviewer.user,:participant_id=>participant,:score=>sprintf("%.2f",score) %>

In our new conflict_email action:

We still need the instructor as the sender

      if session[:user].role_id !=6
        @instructor = session[:user]
      else
        @instructor = Ta.get_my_instructor(session[:user].id)
      end

The reviewer_id is passed from the link,we can query his/her email address to send the email

      recipient=User.find(params[:reviewer_id])

The responsibility of sending email has been delegated to a new mailer conflict_mailer. And the email content was constructed in the mailer.

      participant = AssignmentParticipant.find(params[:participant_id])
      assignment=Assignment.find(participant.parent_id)      
      score=params[:score]
      ConflictMailer.send_conflict_email(instructor,recipient,assignment,score).deliver

Take a look at new conflict_mailer, it define a send_conflict_email method. the email was sent from sender's email and to reviewer's email.Some instance variables were used in view. In conflict_mailer.erb

 def send_conflict_email(sender,recipient,assignment,score)
      @assignment=assignment
      @recipient=recipient
      @score=score
      if recipient.role_id==1
        @role = "reviewer"
        @item = "submission"
      else
        @role = "metareviewer"
        @item = "review"
      end
    mail subject: "Confliction Email",from: sender.email,to: reviewer.email
  end

In its view send_conflict_email.txt.haml, we edit the email content.By this way, we deleted the get_body_text and well followed the rule of MVC design

%h1="Hi ," +@recipient.name

%p="You submitted a score of "+@score+" for assignment "+@assignment.name+" that varied greatly from another "+@role+"'s score for the same "+@item+"."

%p="The Expertiza system has brought this to my attention."


The link "email reviewer" request a ajax,so the current view page will not refresh when the action done

      respond_to do |format|
        format.js
      end

If an email was successfully sent,The instructor should get the alert window by ajax.

view_my_scores method

Summary

We refactored view_my_scores method to grades_show, deleted many unnecessary instance variables along with several review and meta reviews methods inside it. Therefore, the system will only search for scores in the database rather than search for scores and reviews, which wastes a lot of time during the grade show process. After refactoring, the complexity on this decreased from 53 to 35. We also optimize get_scores method to improve the efficiency of showing scores by students and instructors, which cost us most of the time during the project. After refactoring this method, the time cost of views in grade controller decreased by more than 90%.

We have done the following to the original methods:

  • Refactored view_my_scores method to grades_show method.
  • Searched for the key reasons which lead to huge waiting time for getting score.
  • Refactored get_assessments_for method in response_map.rb and lead to more than 90% off the current waiting time.
  • Eliminated the search for reviews during the grades_show method.
  • Deleted unnecessary instance Variables.

Implementation

  • Deleted the unnecessary instance variables

Deleted Code:

 @average_score_results = Array.new
 @average_score_results = ScoreCache.get_class_scores(@participant.id)
 @statistics = Array.new
 @average_score_results.each { |x|
 @statistics << x
 }
  • Deleted query for reviews and meta_reviews

Deleted Code:

 @average_reviews = ScoreCache.get_reviews_average(@participant.id)
 @average_metareviews = ScoreCache.get_metareviews_average(@participant.id)
 @my_reviews = ScoreCache.my_reviews(@participant.id)
 @my_metareviews = ScoreCache.my_metareviews(@participant.id)
  • Modify get_assessments_for method in response_map.rb

After doing this, the time cost of view function decreased by more than 90% Before Refactoring:

 # the original method to find all response 
 @all_resp=Response.all
 for element in @all_resp
     if (element.map_id == map.map_id)
         @array_sort << element
         @test << map
     end
 end

After Refactoring:

 @all_resp=Response.find_by_map_id(map.map_id)
 @array_sort << @all_resp
 @test << map
  • Add a new method into grades_controller.rb

Add Method:

 #show_reviews method gets reviews, meta reviews and author feedback information from server.
  def show_reviews
    @partial='grades/'+params[:path]
    @prefix=params[:prefix]
    @score=Hash.new
    @score[:assessments]=Array.new
    params[:score][:assessments].each do |assessment|
        @score[:assessments]<<Response.find(assessment)
    end
    @score[:scores]=params[:score][:scores]
    @participant=AssignmentParticipant.find(params[:participant])
    @assignment = @participant.assignment
    @questions = {}
    questionnaires = @assignment.questionnaires
    questionnaires.each do |questionnaire|
      @questions[questionnaire.symbol] = questionnaire.questions
    end
  end
  • Change view_my_score.html.erb into show_reviews.html.erb in views.
  • Add in views(show_reviews.html.erb)
 <%= render :partial=>@partial, :locals => {:prefix => @prefix, :participant => @participant, :rscore => @score} %>

Test Result

  • The following test results based on the test case on Expertiza(Assignments: Program 2 style)
Name Before Refactoring After Refactoring Reduced By
View all team's score(instructor) 484988ms 8642ms 98.21%
View own score(student) 8941ms 651ms 92.71%
  • Original Time for Instructor to View all scores
Original Time for Instructor to View all scores
  • Time for Instructor to View all scores after Refactoring
Time for Instructor to View all scores after Refactoring
  • Original Time for Student to View all scores
Original Time for Student to View all scores
  • Time for Student to View all scores after Refactoring
Time for Student to View all scores after Refactoring


See Also