CSC/ECE 517 Spring 2022 - E2217: Refactor questionnaires controller

This page describes the changes made for the Spring 2022 OSS Project E2217: Refactoring Questionnaires Controller

Description

This project focused on refactoring questionnaires_controller.rb by sanding down rough edges of code throughthe 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: create_questionnaire method and its corresponding test has been removed because it was not used anywhere.

Problem: The create method is 49 lines long

• Solution:@questionnaire initialization was moved to a new method called initialize_values. The questionnaire node initialization was moved to create_questionnaire_node.

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: The first check was located in the create_questionnaire and has been deleted along with the method.

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 ScoredQuestion items, constants were created in scored_question.rb. add_new_questions now 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_id was changed to params[:id]. This was done in methods where params[:id].nil? was not called, and in methods where questionnaire_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

questionnaires_controller.rb

scored_question.rb

questionnaires_controller_spec.rb

Testing

Running Tests

  rspec ./spec/controllers/questionnaires_controller_spec.rb

Future Improvements

• Check for if question.is_a? ScoredQuestion has been removed.

• Explore if create_questionnaire_node can be moved to the node class

• Explore moving ScoredQuestion initialization into the scored_question.rb model.

GitHub links and Pull Request

Link to Expertiza repository: here

Link to the forked repository: here

Link to Pull Request: here