CSC/ECE 517 Fall 2015/oss E1566 ARB: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
 
(9 intermediate revisions by 2 users not shown)
Line 19: Line 19:


A controller and a helper file were modified for this project namely:<br/>
A controller and a helper file were modified for this project namely:<br/>
1.GradesController <br/>
1. GradesController <br/>
2.GradesHelper <br/>
2. GradesHelper <br/>


== GradesController ==
=== GradesController ===
   
   
This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style.
This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style.
Line 29: Line 29:
There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.
There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.


== GradesHelper ==
=== GradesHelper ===


This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)
This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)


== List of changes ==
== List of changes ==
We worked on the following work items<br/>
We worked on the following work items(WIs)<br/>
WI1 : Refactor calculate_all_penalties method into smaller methods<br/>
WI1 : Refactor calculate_all_penalties method into smaller methods<br/>
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions.<br/>
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions.<br/>
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices<br/>
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices<br/>
WI4 : Test the conflict_notification method to test the changes made.
WI4 : Test the conflict_notification method to test the changes made.<br/>
WI5 : Move the repeated code in view and view_my_scores methods to a separate method retrieve_questions


== Solutions Implemented and Delivered ==
=== Solutions Implemented and Delivered ===


*WI1 : Refactoring calculate_all_penalties method
*Refactoring calculate_all_penalties method


This is used to calculate various penalty values for each assignment if penalty is applicable.
This is used to calculate various penalty values for each assignment if penalty is applicable.
Line 48: Line 49:
The following changes were made:
The following changes were made:


1.This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function.
1. This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function.
 
2. The following 3 methods were created after splitting the first method<br>
2.The following 3 methods were created after splitting the first method
  i. calculate_all_penalties<br>
i. calculate_all_penalties
  ii. calculate_penatly_attributes<br>
ii. calculate_penatly_attributes
  iii. assign_all_penalties<br>
iii. assign_all_penalties
 
3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly.
3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly.
4. Finally some redundant code was commented out as it was non-functional.
4. Finally some redundant code was commented out as it was non-functional.


Line 67: Line 65:
Removal of non-functional code :
Removal of non-functional code :


Before:
[[File:Change5_new.png]]
                                     
      if(penalties[:submission] != 0 || penalties[:review] != 0 || penalties[:meta_review] != 0)
        unless penalties[:submission]
          penalties[:submission] = 0
        end
        unless penalties[:review]
          penalties[:review] = 0
        end
        unless penalties[:meta_review]
          penalties[:meta_review] = 0
        end
        @total_penalty = (penalties[:submission] + penalties[:review] + penalties[:meta_review])
 
 
After:
 
    if(penalties[:submission] != 0 || penalties[:review] != 0 || penalties[:meta_review] != 0)
        @total_penalty = (penalties[:submission] + penalties[:review] + penalties[:meta_review])
 




Change of language to make it more Ruby friendly:
Change of language to make it more Ruby friendly:


[[File:Change1_new.png]]


Before:
  if(penalties[:submission] != 0 || penalties[:review] != 0 || penalties[:meta_review] != 0)
After:
    unless(penalties[:submission].zero? || penalties[:review].zero? || penalties[:meta_review].zero? )


*WI2 Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions  
*Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions  


The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate.
The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate.
This was again split into 2 methods with some part of the code which is repeated in another method  refactored into a new method.
This was again split into 2 methods with some part of the code which is repeated in another method  refactored into a new method.


Before:
[[File:Change3_new.png]]


  def conflict_notification
    .
    .
    .
    @assignment = Assignment.find(@participant.parent_id)
    @questions = Hash.new
    questionnaires = @assignment.questionnaires
    questionnaires.each {
      |questionnaire|
      @questions[questionnaire.symbol] = questionnaire.questions
    }
    @reviewers_email_hash = Hash.new
    .
    .
    .
  end


After:
Refactored #Created a method which was a duplicate in conflict_notification and edit methods
 
[[File:Change4_new.png]]


    def conflict_notification
edit method:
    .
    .
    .
    @assignment = Assignment.find(@participant.parent_id)
    #Refactored--> Replaced with a methood call
    list_questions (@assignment)
    @reviewers_email_hash = Hash.new
    .
    .
    .
  end


This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.


#Refactored #Created a method which was a duplicate in conflict_notification and edit methods
[[File:Change2_new.png]]
  def list_questions (assignment)
    @questions = Hash.new
    questionnaires = assignment.questionnaires
    questionnaires.each {
        |questionnaire|
      @questions[questionnaire.symbol] = questionnaire.questions
    }
  end


edit method:
New method:
Refactored #Created a method which was a duplicate in conflict_notification and edit methods
 
[[File:Change4_new.png]]


This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.
Similar refactoring was performed to obtain the retrieve_questions method:


Before:
[[File:Latest1.png]]
    def edit
    @participant = AssignmentParticipant.find(params[:id])
    @assignment = @participant.assignment
    @questions = Hash.new
    questionnaires = @assignment.questionnaires
    questionnaires.each {
      |questionnaire|
      @questions[questionnaire.symbol] = questionnaire.questions
    }
    @scores = @participant.scores(@questions)
  end


After:
This is the new method created after the above refactoring:
    def edit
    @participant = AssignmentParticipant.find(params[:id])
    @assignment = @participant.assignment
    #Refactored--> Replaced with a methood call
    list_questions (@assignment)
    @scores = @participant.scores(@questions)
  end


New method:
[[File:Latest2.png]]
#Refactored #Created a method which was a duplicate in conflict_notification and edit methods
  def list_questions (assignment)
    @questions = Hash.new
    questionnaires = assignment.questionnaires
    questionnaires.each {
        |questionnaire|
      @questions[questionnaire.symbol] = questionnaire.questions
    }
  end


== Testing Details==
== Testing Details==


There were no exiting test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method.
=== RSpec ===
 
There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything.
As the model was not changed, no test cases were added for the model.


=== UI Testing ===
=== UI Testing ===
Line 205: Line 123:
9. Give reviews on first student's work.<br/>
9. Give reviews on first student's work.<br/>
10. Login as instructor or first student to look at the review grades.<br/>
10. Login as instructor or first student to look at the review grades.<br/>
== Scope for future improvement ==
1. The construct_table method in GradesHelper is not used anywhere. It has no reference in the project. So we feel it can be safely removed.<br/>
2. The has_team_and_metareview? method in GradesHelper can be broken down into separate methods, one each for team and metareview. This will provide improved flexibility. It needs some analysis though, as both the entities(team & metareview) are currently checked in conjuction from all the views they are referenced from.

Latest revision as of 01:57, 10 November 2015

This wiki page is for the description of changes made under E1555 OSS assignment for Fall 2015, CSC/ECE 517.

Peer Review Information

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:

  • Instructor login: username -> instructor6, password -> password
  • Student login: username -> student4340, password -> password
  • Student login: username -> student4405, password -> password

Expertiza Background

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NCSU. It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It allows students to review each other’s work and improve their work upon this feedback.

Description of the current project

The following is an Expertiza based OSS project which deals primarily with the GradesController and GradesHelper. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. The goal of this project is to attempt to make this part of the application easier to read and maintain.

Files modified in current project

A controller and a helper file were modified for this project namely:
1. GradesController
2. GradesHelper

GradesController

This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style. Three methods in particular, namely conflict_notification ,calculate_all_penalties and edit were found to be too long and were in need of refactoring into smaller, easier to manage methods. Few more compact methods were created for this purpose.

There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.

GradesHelper

This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)

List of changes

We worked on the following work items(WIs)
WI1 : Refactor calculate_all_penalties method into smaller methods
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions.
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices
WI4 : Test the conflict_notification method to test the changes made.
WI5 : Move the repeated code in view and view_my_scores methods to a separate method retrieve_questions

Solutions Implemented and Delivered

  • Refactoring calculate_all_penalties method

This is used to calculate various penalty values for each assignment if penalty is applicable.

The following changes were made:

1. This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function. 2. The following 3 methods were created after splitting the first method

  i.  calculate_all_penalties

  ii. calculate_penatly_attributes

  iii. assign_all_penalties

3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly. 4. Finally some redundant code was commented out as it was non-functional.


Refactoring into smaller more specific methods:

Removal of non-functional code :


Change of language to make it more Ruby friendly:


  • Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions

The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate. This was again split into 2 methods with some part of the code which is repeated in another method refactored into a new method.


Refactored #Created a method which was a duplicate in conflict_notification and edit methods

edit method:

This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.

New method: Refactored #Created a method which was a duplicate in conflict_notification and edit methods

Similar refactoring was performed to obtain the retrieve_questions method:

This is the new method created after the above refactoring:

Testing Details

RSpec

There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything. As the model was not changed, no test cases were added for the model.

UI Testing

Following steps needs to be performed to test this code from UI:
1. Login as instructor. Create a course and an assignment under that course.
2. Keep the has team checkbox checked while creating the assignment. Add a grading rubric to it. Add at least two students as participants to the assignment.
3. Create topics for the assignment.
4. Sign in as one of the students who were added to the assignment.
5. Go to the assignment and sign up for a topic.
6. Submit student's work by clicking 'Your work' under that assignment.
7. Sign in as a different student which is participant of the assignment.
8. Go to Assignments--><assignment name>-->Others' work (If the link is disabled, login as instructor and change the due date of the assignment to current time).
9. Give reviews on first student's work.
10. Login as instructor or first student to look at the review grades.


Scope for future improvement

1. The construct_table method in GradesHelper is not used anywhere. It has no reference in the project. So we feel it can be safely removed.
2. The has_team_and_metareview? method in GradesHelper can be broken down into separate methods, one each for team and metareview. This will provide improved flexibility. It needs some analysis though, as both the entities(team & metareview) are currently checked in conjuction from all the views they are referenced from.

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2015/oss_E1566_ARB&oldid=99287"