CSC/ECE 517 Fall 2016/E1642. Refactor review response map.rb
Peer Review Information
For testing the changes made, the following credentials are recommended:
- Instructor Login: username: instructor6 password: password
- Student Login: username: student11 password: password
- Student Login: username: student1862 password: password
The above users are suggested for testing because there are a few users which would lead to exceptions upon login for unknown reasons completely unrelated to our work.
Introduction to Expertiza
Background
Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
Problem Statement
The following were the tasks identified to accomplish through this project. These tasks are part of the review_response_map.rb, assignment.rb and metareview_response_maps.
- P1: Code Climate shows import method is complex, because of lots of checks. This method can be fixed by adding private methods for raising import error. However, you need to check if this functionality is called anywhere.
- P2: Get_assessments_round_for method can be renamed to get_responses_for_team_round. Team_id private variable is not needed.
- P3: metareview_response_maps rename, refactoring can be done. No need to do second iteration.
- P4: Assignment Branch Condition size for final_versions_from_reviewer is too high.
- P5: Assignment Branch Condition size for get_assessments_round_for is too high, try to get rid of the nested if.
- P6: Use find_by instead of where.first, e.g. line 60, 65, etc.
- P7: Wrap the over-long lines: 58, 62 and 181.
- P8: Write missing unit tests for existing methods.
Reason for Refractor
The purpose of this work is to rewrite the code of seven file for several reason listed below.
Files Modified in this Project
- review_response_map.rb
- assignment.rb
- metareview_response_maps
Solutions implemented and Delivered
P1:
We could find a call to the import method from 4 relevant places in the importFile method in the ImportFileController. Of them 2 are for different models and the other 2 are having different number of parameters. Therefore we concluded that the import method in the review_response_map.rb is not called from anywhere and so we dropped the method.
P2:
- Get_assessments_round_for Method is refactored to get_responses_for_team_round as required. In order to test from UI follow the steps:
- Login as instructor6
- Click on Manage... tab and then on the Assignments section
- Go on Wikipedia contribution assignment and then click on the View scores button.(This will call the above method)
- Then for any one team, click on Alternate View (This is another call to the method via a different route.)
P3:
There is no need for the inner iteration as for every review for a particular round, there is only one metareview.
P4:
Assignment Branch Condition size for final_versions_from_reviewer is too high:
This function returns a map containing the final versions of a reviewer. The function has duplicate code for scenario with varying rubric and non varying rubric. We moved this code into a new function with method name prepare_review_response. As we removed the duplicate code, this will reduce the Assignment Branch Condition size.
In order to test from UI follow the steps:
- Login as instructor6
- Click on Manage -> tab ->Assignments section
- Click on view review report
- Click on review summary
P6:
find_by method can be used instead of where.first. Although here it wasn't used since the lines of code that used where.first were redundant and were removed to solve other problems. But its application can found on this page http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find_by
P7:
Over long lines can be hard to read in a small screen. Hence proper use of refactoring the lines can save the overhead of reading and understanding long lines of code. In this problem we have wrapped the overlong lines. In an IDE such as RubyMine we can use custom methods such as "Use Soft Wraps in editor" to ease the process.
P8:
==