CSC/ECE 517 Spring 2021 - E2103. Refactor response controller.rb: Difference between revisions
m (Skeleton) |
No edit summary |
||
| Line 3: | Line 3: | ||
== Background == | == Background == | ||
A response is the object that is created when someone fills out a review rubric, such as when one writes a review, gives feedback to a reviewer, or fills out a survey. Responses to the individual rubric items are kept in Answer objects; each Answer object has a response_id to say what Response it is part of. Since response_controller needs to work with many kinds of responses, its code is pretty general. It is not the worst controller in the system, but it would be much clearer if its method names were more descriptive of what they do. | |||
=== Mentor === | === Mentor === | ||
Ed Gehringer, efg@ncsu.edu | |||
=== Team Members === | === Team Members === | ||
== Issues == | == Issues == | ||
* '''def assign_instance_vars''' This name could be more specific. | |||
* '''def scores''' This should be a model method (in response.rb)! It is all calculation. | |||
* '''def new''' This method contains a complicated condition that determines whether the submission has been updated since the last time it was reviewed. If it has not, then the reviewer can edit his/her previous review. If there has been an update, then the reviewer gets a new review form to “update” the review. It would make sense to have a model method that tests whether there has been a review since the last file or link was submitted. Then the code here would just call that function. It would be a lot clearer what the new method is doing. set_content(new_response = false) also plays a role in this calculation. Perhaps this method should also be included in the refactoring, for the sake of clarity of the resulting code. | |||
* '''def set_questionnaire''' Badly named; what kind of questionnaire and why? The name should be a lot clearer. | |||
* '''def set_questionnaire_for_new_response''' Badly named, no comments, not at all clear. The logic is not like set_questionnaire. Rename, refactor for clarity, and add comments as appropriate. | |||
* '''def show_calibration_results_for_student''' This method makes about five db accesses. Can it be broken into 2 methods, with the business logic moved to response.rb? | |||
Revision as of 01:37, 18 March 2021
This page details project documentation for the CSC/ECE 517 Spring 2021, "E2103 refactor response_controller.rb" project.
Background
A response is the object that is created when someone fills out a review rubric, such as when one writes a review, gives feedback to a reviewer, or fills out a survey. Responses to the individual rubric items are kept in Answer objects; each Answer object has a response_id to say what Response it is part of. Since response_controller needs to work with many kinds of responses, its code is pretty general. It is not the worst controller in the system, but it would be much clearer if its method names were more descriptive of what they do.
Mentor
Ed Gehringer, efg@ncsu.edu
Team Members
Issues
- def assign_instance_vars This name could be more specific.
- def scores This should be a model method (in response.rb)! It is all calculation.
- def new This method contains a complicated condition that determines whether the submission has been updated since the last time it was reviewed. If it has not, then the reviewer can edit his/her previous review. If there has been an update, then the reviewer gets a new review form to “update” the review. It would make sense to have a model method that tests whether there has been a review since the last file or link was submitted. Then the code here would just call that function. It would be a lot clearer what the new method is doing. set_content(new_response = false) also plays a role in this calculation. Perhaps this method should also be included in the refactoring, for the sake of clarity of the resulting code.
- def set_questionnaire Badly named; what kind of questionnaire and why? The name should be a lot clearer.
- def set_questionnaire_for_new_response Badly named, no comments, not at all clear. The logic is not like set_questionnaire. Rename, refactor for clarity, and add comments as appropriate.
- def show_calibration_results_for_student This method makes about five db accesses. Can it be broken into 2 methods, with the business logic moved to response.rb?
Code Refactor Tasks and Issues
Below is a list of code refactor tasks and issues addressed by this team in this project. We have also included any pertinent details, reasonings, comments, warnings, etc., corresponding to each task.
METHOD: def scores
This method performs a task that is already defined and working in other classes. After careful consideration, this particular method was removed.
- add picture?