CSC/ECE 517 Spring 2022 - E2217: Refactor questionnaires controller: Difference between revisions
No edit summary |
No edit summary |
||
| Line 9: | Line 9: | ||
*'''Solution''': | *'''Solution''': | ||
==== '''Problem''': The <code>create</code> method is 49 lines long | ==== '''Problem''': The <code>create</code> method is 49 lines long==== | ||
*'''Solution''': | *'''Solution''': | ||
===='''Problem''': There are three checks for whether a question is a <code>QuizQuestionnaire</code> | ===='''Problem''': There are three checks for whether a question is a <code>QuizQuestionnaire</code>==== | ||
These checks are likely unnecessary, and testing the class of an object carries potential problems. | |||
*'''Solution''': | *'''Solution''': | ||
===='''Problem''': There are assignment values hardwired into the code in <code>add_new_questions</code> | ===='''Problem''': There are assignment values hardwired into the code in <code>add_new_questions</code>==== | ||
Although these fields are okay to default to the given values, these should be defined constants | |||
*'''Solution''': As these values related to <code>ScoredQuestion</code> items, constants were created in <code>scored_question.rb</code>. <code>add_new_questions</code> now uses these class constants when assigning default values for fields. These values are all changeable in the UI after initialization. | *'''Solution''': As these values related to <code>ScoredQuestion</code> items, constants were created in <code>scored_question.rb</code>. <code>add_new_questions</code> now uses these class constants when assigning default values for fields. These values are all changeable in the UI after initialization. | ||
| Line 22: | Line 24: | ||
[[File:Scored_question_changes.png]] | [[File:Scored_question_changes.png]] | ||
===='''Problem''': Several methods have a <code>questionnaire_id</code> as a parameter, this may be unnecessary in several situations where <code>params[:id]</code> would suffice | ===='''Problem''': Several methods have a <code>questionnaire_id</code> as a parameter==== | ||
Although not functionally broken, this may be unnecessary in several situations where <code>params[:id]</code> would suffice | |||
*'''Solution''': Where applicable, <code>questionnaire_id</code> was changed to <code>params[:id]</code>. This was done in methods where <code>params[:id].nil?</code> was not called, and in methods where <code>questionnaire_id = params[:id]</code> was called | *'''Solution''': Where applicable, <code>questionnaire_id</code> was changed to <code>params[:id]</code>. This was done in methods where <code>params[:id].nil?</code> was not called, and in methods where <code>questionnaire_id = params[:id]</code> was called | ||
Revision as of 22:59, 20 March 2022
This page describes the changes made for the Spring 2022 OSS Project E2217: Refactoring Questionnaires Controller
Description
This project focused on sanding down the rough edges of questionnaires_controller.rb through the removal of the unused create_questionnaire method, breaking up the exceedingly long create method, transfer of hardcoded values in the add_new_questions method, and general code cleanup.
Problems and Solutions
Problem: The create_questionnaire method is not used
create_questionnaire has no immediate apparent calls, and appears to have the same functionality as create
- Solution:
Problem: The create method is 49 lines long
- Solution:
Problem: There are three checks for whether a question is a QuizQuestionnaire
These checks are likely unnecessary, and testing the class of an object carries potential problems.
- Solution:
Problem: There are assignment values hardwired into the code in add_new_questions
Although these fields are okay to default to the given values, these should be defined constants
- Solution: As these values related to
ScoredQuestionitems, constants were created inscored_question.rb.add_new_questionsnow uses these class constants when assigning default values for fields. These values are all changeable in the UI after initialization.
Addition of constants to scored_question.rb:
Problem: Several methods have a questionnaire_id as a parameter
Although not functionally broken, this may be unnecessary in several situations where params[:id] would suffice
- Solution: Where applicable,
questionnaire_idwas changed toparams[:id]. This was done in methods whereparams[:id].nil?was not called, and in methods wherequestionnaire_id = params[:id]was called
Change of questionnaire_id to params[:id] in save_all_questions:
Problem: Several bad if statements exist
- Solution: Line 96:
if @questionnaire.type != "QuizQuestionnaire"checking if it is not a QuizQuestionnaire is not necessary so this line has been removed because the QuizQuestionnaire is same as the other questionnaire types.
Line 244 & 266 : if @questionnaire.type != "QuizQuestionnaire" checking if it is not a QuizQuestionnaire is not necessary so this line has been removed because the for saving a new question there is no need to check for the QuizQuestionnaire and weights used in this check are not used anywhere.
Line 190 : if question.is_a? ScoredQuestion check for ScoredQuestion has been removed.Also hardcoded the minlabel and max label values from question.max_label = 'Strongly agree' question.min_label = 'Strongly disagree' to
question.max_label = ScoredQuestion::DEFAULT_MAX_AGREEMENTquestion.min_label = ScoredQuestion::DEFAULT_MIN_AGREEMENT
Modified Files
(Add modified files and list of changes here)
Testing
Running Tests
rspec ./spec/controllers/questionnaires_controller_spec.rb
Future Improvements
check for if question.is_a? ScoredQuestion has been removed.explore moving ScoredQuestion initialization into the scored_question model
Pull Request
GitHub link: https://github.com/expertiza/expertiza/pull/2320