CSC/ECE 517 Spring 2023 -E2326 Refactor questionnaires controller.rb
Background
In Expertiza, Questionnaire is the superclass for all kinds of questionnaires and rubrics—rubrics for evaluating submissions and teammate contributions, and taking quizzes and surveys. All of these are subclasses of Questionnaire. questionnaires_controller.rb is charged with creating, displaying, and managing Questionnaires. Because it is used so widely, malfunctions could cause bugs in many parts of the system; hence it is especially important that the code be clear.
Project Purpose
In recent years, questionnaires_controller.rb has been refactored repeatedly. It is a lot clearer than it used to be. But rough edges still remain. question.rb contains a large number of constants. It is not clear what they are used for. There is a questionnaires_controller, but no questions_controller. The questionnaires_controller creates Question objects. This is not elegant, because a controller should only create objects of one type, the type of objects it controls.
Proposed Design Changes
| # | File/Function | Change Proposed | Rationale | Commit Link |
|---|---|---|---|---|
| 1 | method add_new_questions()
|
Store the used default value to some kind of constant in questions.rb, so that they can be changed without refactoring code
|
There are many literals used in the code and can change in future, so it is better to separate them | Will be added later |
| 2 | File questionnaires_controller.rb
|
Move save_all_questions(), delete_questions(), save_new_questions(), save_questions() | To be consistent with SOLID principle, these methods belong in questions_controller.rb | Will be added later |
| 3 | file questionnaires_controller.rb
|
Refactor following vague method names | There are confusing function names like save_all_questions() , save_new_questions() , save_questions() and save()
|
Will be added later |
| 4 | method save_questions()
|
Change params[:question].keys.each to params[:question].each_key
|
This will improve the performance and uses a single enumerator | Will be added late |
| 5 | method create()
|
Remove parenthesis around function call on line 72 | Unnecessary parenthesis | Will be added later |
| 5 | File questionnaires_controller.rb
|
Refactor large methods | Create, update, and delete are noticeably larger than most methods | Will be added later |
| 6 | method save_all_questions()
|
Find a way to refactor the code by using more variables or Procs that can be reused in questions_controller.rb
|
The process of saving all questions with 2 nested do blocks is difficult to understand | Will be added later |
| 6 | method create | Move code related to creation of tree node to different function name create_tree_node()
|
The create method is very big and this would help in readability and code reusability | Will be added later |
| 7 | method questionnaire_helper()
|
Change if/else to switch case and add default error checking to helper | Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now. | Will be added later |
| 8 | method question_helper()
|
Change if/else to switch case and add default error checking to helper | Code will be cleaner and there will needs to be catch all because there are no default else conditions/ error checks in factories as of now. | Will be added later |
Important links
Github: https://github.com/Soultrekker21/expertiza
Test Plan
- Make sure that current proposed changes don't break existing test cases.
- Add unit test for newly proposed methods.
- Shift existing test cases for questionnaire_controller_spec.rb to question_controller.rb corresponding to methods that are being shifted