|
|
| Line 1: |
Line 1: |
| ==Introduction== | | ==Introduction== |
| This page gives a description of the changes made for the ''questionnaires_controller.rb'' of Expertiza based OSS project. | | This page gives a description of the changes made for the ''questionnaires_controller.rb'' of the Expertiza-based OSS project. |
|
| |
|
| ===Expertiza Background === | | ===Expertiza Background === |
| Expertiza is a web application where students can submit and peer-review learning objects (articles, codes, websites, etc). Instructors add and grade the assignments submitted by students to Expertiza. Students can be assigned in teams based on their selection of the topics. It has functionalities such as peer reviews in which students can provide feedback on other's work which helps peer in better developing the project. It is supported by the National Science Foundation. | | Expertiza is a web application where students can submit and peer-review learning objects (articles, codes, websites, etc). Instructors add and grade the assignments submitted by students to Expertiza. Students can be assigned in teams based on their selection of the topics. It has functionalities such as peer reviews in which students can provide feedback on others' work which helps peer in better developing the project. It is supported by the National Science Foundation. |
|
| |
|
| === Problem Statement === | | === Problem Statement === |
|
| |
|
| <b>What is needed:</b> This project builds on E2125, and should begin with the refactoring done by that project. That project focused on simplifying the methods in review_mapping_helper, while this project looks at making the code more understandable and transparent. | | <b>What is needed:</b> This project builds on E2257. That project focused on simplifying the methods in questionnaires_controller, while this project's objective is to separate the current controller into two different controllers - questionnaires_controller and questions_controller. Further cleaning of code is also planned along with this. |
|
| |
|
| * Method <b><i>get_data_for_review_report</i></b> marshals a lot of data together and passes back a data structure. It is used in views/reports/_review_report.html.erb, but it is quite difficult to see how the data is being displayed. It violates the <b>Expert pattern</b>, because the view needs to know how to break apart the structure that is passed to it. Doing the same thing with partials in views/reports would make the code easier to follow | | * Make use of temporary variables instead of instance variables wherever possible to narrow down their scope. |
| * The next three methods involve team <i>“color”</i>. Color-coding is explained on this page and this page for the E1789 project and this page for E1815. The code for these methods is not at all clear, and should be refactored. And please use the American spelling “color”.
| | * question.rb contains a large number of constants. Make clear comments on their usage else remove it. |
| * Various method names begin with <b><i>get</i></b>. This is not Ruby-like. Change the names to something more appropriate. | | * Since a controller should only create objects of one type the questions_controller could be introduced to create and edit questions moving the logic of dealing with individual questions into a separate controller, and improving the separation of responsibility. |
| * The <b><i>get_awarded_review_score</i></b> computes an overall score based upon scores awarded in individual rounds. This is one of many places in Expertiza where scores are being calculated. Score-calculation code for multiple rounds is being standardized now, in response_map.rb. Contact Nicholas Himes (nnhimes@ncsu.edu) for particulars. Change this method to use the new code.
| |
| * The method <b><i>sort_reviewer_by_review_volume_desc</i></b> should be generalized so that it can sort by any metric, not just review volume. Other metrics might include number of suggestions, or number of suggestions + number of problems detected. This method should not be counting the number of review rounds! Since other places in the code will need to know the number of review rounds, it should be calculated somewhere else in the system.
| |
| * The next several methods generate charts. They are cohesive enough that they should be in their own separate file, either another helper or a mixin. | |
| * There is a method <b><i>list_review_submissions</i></b>. It’s not at all clear that this method is needed, though it is used in one view. Look on the Expertiza wiki and see if there is a better way.
| |
| * Also there are several methods for <b><i>feedback_response_maps</i></b>. It is not at all clear why they are here. Look on the Expertiza wiki for the documentation, and see if you can replace them by calls to other methods, or at least make it clearer what they are doing.
| |
| * The file ends with three small classes being defined. There are no comments at all to explain what is being done. Look them up on the Expertiza wiki and refactor or comment them, whichever seems more appropriate.
| |
|
| |
|
| ==Implementation== | | ==Implementation== |
| Line 31: |
Line 25: |
|
| |
|
| ====Refactoring method==== | | ====Refactoring method==== |
| All the method names that are refactored are mentioned below-
| |
|
| |
| review_ get_data_for_review_report
| |
|
| |
| Since this method marshals a lot of data together and passes back a data structure, it is quite difficult to see how the data is being displayed. Therefore what we were thinking is to remove this existing method, create new partials which renders specific data for each individual tasks performed in the current method and pass it to the views file. Similar to what we do Expert pattern.
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| get_team_colour
| |
|
| |
| Since the method is not particularly clear we will be refactoring the method in such a way that no code is repeated, no conditions is checked multiple times and finally move this piece of code to review_response_maps.rb to decrease the total length of the file (>450 line).
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| To remove 'get' from the below methods to something ruby like methods( below suggestions are subject to change)
| |
|
| |
| get_data_for_review_report to data_for_review_report
| |
| get_team_colour to team_colour
| |
| get_link_updated_at to link_updated_at
| |
| get_team_reviewed_link_name to team_reviewed_link_name
| |
| get_awarded_review_score to awarded_review_score
| |
| get_review_metrics to review_metrics
| |
| get_each_review_and_feedback_response_map to review_and_feedback_response_map
| |
| get_certain_review_and_feedback_response_map to certain_review_and_feedback_response_map
| |
|
| |
| Since methods starting with the the word 'get' are not ruby-esque and may cause unintended side-effects, we'll be changing the method names
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| get_awarded_review_score
| |
|
| |
| This method computes an overall score based upon scores awarded in individual rounds. Since this task is already being done in response_map.rb, we'll using this rb file's method and remove the repeated method from this file.
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| sort_reviewer_by_review_volume_desc
| |
|
| |
| This method is currently sorting the reviewer using review volume. We'll be generalizing this method such that it takes 1 more additional metrics and sort reviewers using the provided metrics
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| The below methods
| |
|
| |
| initialize_volume_metric_chart_elements
| |
| display_volume_metric_chart
| |
| initialize_review_metrics_chart_elements
| |
| display_review_metrics_chart
| |
| prepare_chart_data
| |
| prepare_chart_options
| |
|
| |
| These methods are specific to to creating and editing charts so they should have their own helper/mixin so we'll be moving these methods out of review_mapping_helper.rb to some other helper/mixin.
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| list_review_submissions
| |
|
| |
| We'll be understanding and refactoring this method as currently its not known where this method is being used. If we find that this method is not being used, then we'll remove it from the file.
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| The below methods
| |
|
| |
| get_each_review_and_feedback_response_map
| |
| get_certain_review_and_feedback_response_map
| |
|
| |
| These methods use 'feedback_response_maps' and its not clear how and where they are used. So our task is to find out how and where these methods are being used and how we can implement it better.
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
|
| |
| The below classes
| |
|
| |
| class ReviewStrategy
| |
| attr_accessor :participants, :teams
| |
| def initialize(participants, teams, review_num)
| |
| @participants = participants
| |
| @teams = teams
| |
| @review_num = review_num
| |
| end
| |
| end
| |
|
| |
| class StudentReviewStrategy < ReviewStrategy
| |
| def reviews_per_team
| |
| (@participants.size * @review_num * 1.0 / @teams.size).round
| |
| end
| |
| def reviews_needed
| |
| @participants.size * @review_num
| |
| end
| |
| def reviews_per_student
| |
| @review_num
| |
| end
| |
| end
| |
|
| |
| class TeamReviewStrategy < ReviewStrategy
| |
| def reviews_per_team
| |
| @review_num
| |
| end
| |
| def reviews_needed
| |
| @teams.size * @review_num
| |
| end
| |
| def reviews_per_student
| |
| (@teams.size * @review_num * 1.0 / @participants.size).round
| |
| end
| |
| end
| |
|
| |
| Its not known where are being used so we'll be removing these classes or provide better documentation for these classes if their application is found.
| |
|
| |
|
| |
| ------------------------------------------------------------------------------------------------------------------------
| |
|
| |
| ===Function placed in the view as partials===
| |
|
| |
| The following method has been removed from the '''review_mapping_helper.rb''' and added to the views as partials:
| |
|
| |
| <pre>
| |
| def create_report_table_header(headers = {})
| |
| table_header = "<div class = 'reviewreport'>\
| |
| <table width='100% cellspacing='0' cellpadding='2' border='0' class='table table-striped'>\
| |
| <tr bgcolor='#CCCCCC'>"
| |
| headers.each do |header, percentage|
| |
| table_header += if percentage
| |
| "<th width = #{percentage}>\
| |
| #{header.humanize}\
| |
| </th>"
| |
| else
| |
| "<th>\
| |
| #{header.humanize}\
| |
| </th>"
| |
| end
| |
| end
| |
| table_header += "</tr>"
| |
| table_header.html_safe
| |
| end
| |
| </pre>
| |
|
| |
|
| |
| The following is the code available in views in the file '''_report_table_header.html.erb''':
| |
|
| |
| <pre>
| |
| <% table_header = "<div class = 'reviewreport'><table width='100% cellspacing='0' cellpadding='2' border='0' class='table table-striped'><tr bgcolor='#CCCCCC'>" %>
| |
| <% headers.each do |header, percentage| %>
| |
| <% table_header += if percentage %>
| |
| <% "<th width = #{percentage}> #{header.humanize} </th>" %>
| |
| <% else %>
| |
| <%"<th> #{header.humanize} </th>" %>
| |
| <% end %>
| |
| <% end %>
| |
| <% table_header += "</tr>"%>
| |
| <% return table_header.html_safe %>
| |
| </pre>
| |
|
| |
|
| ==Testing== | | ==Testing== |
Introduction
This page gives a description of the changes made for the questionnaires_controller.rb of the Expertiza-based OSS project.
Expertiza Background
Expertiza is a web application where students can submit and peer-review learning objects (articles, codes, websites, etc). Instructors add and grade the assignments submitted by students to Expertiza. Students can be assigned in teams based on their selection of the topics. It has functionalities such as peer reviews in which students can provide feedback on others' work which helps peer in better developing the project. It is supported by the National Science Foundation.
Problem Statement
What is needed: This project builds on E2257. That project focused on simplifying the methods in questionnaires_controller, while this project's objective is to separate the current controller into two different controllers - questionnaires_controller and questions_controller. Further cleaning of code is also planned along with this.
- Make use of temporary variables instead of instance variables wherever possible to narrow down their scope.
- question.rb contains a large number of constants. Make clear comments on their usage else remove it.
- Since a controller should only create objects of one type the questions_controller could be introduced to create and edit questions moving the logic of dealing with individual questions into a separate controller, and improving the separation of responsibility.
Implementation
Flowchart
The following process is carried out to complete the project-
Refactoring
Refactoring method
Testing
RSpec Testing
RSpec testing was carried out for the methods that had been refactored. Given below are the tests which are written for the functions that are modified:
describe 'My Test Cases' do
before(:each) do
create(:instructor)
create(:role_of_student)
login_as("instructor6")
visit '/tree_display/list'
click_link 'View reports'
expect(page).to have_current_path('/reports/response_report')
click_link 'View'
end
it "can display review metrics", js: true do
expect(page).to have_content('Metrics')
end
it "can display review grades of each round", js: true do
expect(page).to have_content('Score awarded')
end
it "can display review summary", js: true do
expect(page).to have_content('Reviews done')
end
it "can display review summary", js: true do
expect(page).to have_content('Reviewer')
end
it "can display review summary", js: true do
expect(page).to have_content('Team reviewed')
end
end
Manual Testing
To be carried out.
Design Pattern
A design pattern is a general repeatable solution to a commonly occurring problem in software design. It is a description or template for how to solve a problem that can be used in many different situations.
During the process of refactoring methods as well as method names,Strategy Pattern was used in the implementation. The Strategy pattern is most useful when you want to provide multiple ways of processing a request, without hard-coding knowledge about those different methods into the object that handles the request.
References
Team Members
