CSC/ECE 517 Fall 2015/oss E1569 JNR: Difference between revisions
No edit summary |
No edit summary |
||
| Line 19: | Line 19: | ||
* Method ''delete_participant'' is not used | * Method ''delete_participant'' is not used | ||
* Method ''list_sortable'' is not used | * Method ''list_sortable'' is not used | ||
* Method '' | * Method ''automatic_review_mapping_strategy'' is too long | ||
* Method ''review_report'' has SQL-like code | * Method ''review_report'' has SQL-like code | ||
* Method ''add_user_to_assignment'' should not be in this controller | * Method ''add_user_to_assignment'' should not be in this controller | ||
| Line 43: | Line 43: | ||
This function was confirmed to not be used, since there is no other function in the application that calls it. Therefore, it was removed. | This function was confirmed to not be used, since there is no other function in the application that calls it. Therefore, it was removed. | ||
=== | === automatic_review_mapping_strategy === | ||
This function was originally too long. Hence, we modularised it by moving the part of the code that loops over all teams and creates the ReviewResponseMap into a new function. | |||
=== response_report === | === response_report === | ||
| Line 52: | Line 53: | ||
=== get_team_from_submission === | === get_team_from_submission === | ||
This method is responsible for returning the team given an assignment submission. It did not have any direct correlation with the reviewer/reviewee. Hence, it was moved to the AssignmentTeam model (assignment_team.rb) file as suggested. | |||
=== delete_all_reviewers === | === delete_all_reviewers === | ||
Revision as of 06:01, 31 October 2015
E1569: Refactoring ReviewMappingController
This page provides a description of our Expertiza based OSS project, which aimed at refactoring the ReviewMappingController.
Introduction
Expertiza is an Open Source software tool developed at NC State University. It is used to facilitate assignment and course management. It is primarily intended to facilitate assignments being peer reviewed. It is written as a Ruby on Rails application, thus functioning natively in a web environment. It can be cloned from GitHub.
Refactoring
Refactoring is a process designed to change code without modifying the functionality. Refactoring can improve the readability and the logical design of the software, making sure everything is in the right place and has the right name. This allows code to be understood more quickly by a developer, which shortens the time it takes to develop new features.
Development environment setup
<ref>http://wiki.expertiza.ncsu.edu/index.php/Main_Page</ref>
Problem statement
The ReviewMappingController file in this application (review_mapping_controller.rb) is responsible for setting up mappings between reviewers and reviewees. It essentially connects a reviewer to an assignment. However, it is a fairly bloated file since it has to handle functionality for five different kinds of maps (review response, author feedback, teammate review, meta-review, and quiz response). It is a prime choice for refactoring because the refactoring can clarify the intent of the different methods in the file.
According to the problem statement<ref>https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus/edit</ref>, there were 11 tasks involved in refactoring the review_mapping_controller:
- Method delete_rofreviewer should do the same thing as delete_metareviewer
- Method delete_participant is not used
- Method list_sortable is not used
- Method automatic_review_mapping_strategy is too long
- Method review_report has SQL-like code
- Method add_user_to_assignment should not be in this controller
- Method get_team_from_submission should not be in this controller
- Method delete_all_reviewers doesn’t actually delete all the reviewers, but just the outstanding review response maps. We want to keep this functionality
- Method delete_participant should not be in this controller
- Method name release_reservation doesn’t describe the functionality well
- Method delete_review is not used
Design Approach
Refactoring
guidelines <ref>https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit#</ref> Single Responsibility Principle <ref>https://en.wikipedia.org/wiki/Single_responsibility_principle</ref>
delete_rofreviewer
This function has duplicate functionality to another function, delete_metareviewer, and the other functional is named much better. The method delete_rofreviewer was removed.
delete_participant
This function was confirmed to not be used, since there is no other function in the application that calls it. Therefore, it was removed.
list_sortable
This function was confirmed to not be used, since there is no other function in the application that calls it. Therefore, it was removed.
automatic_review_mapping_strategy
This function was originally too long. Hence, we modularised it by moving the part of the code that loops over all teams and creates the ReviewResponseMap into a new function.
response_report
To make this code easier to read for a pure Ruby developer, the SQL-like code was rewritten with ActiveRecord.
add_user_to_assignment
This method does not belong in this controller, and it is only ever called by participants_helper. Therefore, the method should go into participants_controller, which is where it was moved.
get_team_from_submission
This method is responsible for returning the team given an assignment submission. It did not have any direct correlation with the reviewer/reviewee. Hence, it was moved to the AssignmentTeam model (assignment_team.rb) file as suggested.
delete_all_reviewers
Since the only problem with this method is that the name does not describe the actual or intended functionality, we replaced all instances of “delete_all_reviewers” with “delete_outstanding_reviewers”.
delete_participant
This function was confirmed to not be used, since there is no other function in the application that calls it. Therefore, it was removed.
release_reservation
Since the method name does not accurately describe the functionality within, all instances of “release_reservation” were replaced with “release_mapping”.
delete_review
This function was confirmed to not be used, since there is no other function in the application that calls it. Therefore, it was removed.
Test Case Development
Conclusion
By refactoring review_mapping_controller, extraneous and unused code was removed, readability was increased, and tests confirm that its functionality remained intact. Users will experience incrementally better performance, and developers will have an easier time getting up to speed on the software, resulting in heightened developer efficiency and faster turnaround times for new features.
Project Links
References
<references/>