CSC/ECE 517 Spring 2021 - E2101. Refactor questionnaires controller.rb

From Expertiza_Wiki
Revision as of 02:39, 20 March 2021 by Jwhostet (talk | contribs)
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)
Jump to navigation Jump to search

Team members

  • Waseem Hanna
  • JP Villamor
  • John Wesley Hostetter

Introduction

There are various types of questionnaires that a user can create in Expertiza. These questionnaires assist in evaluating submissions and teammate contributions.

Problem statement

Questionnaire is a superclass to many different types of questionnaires offered in Expertiza. The Questionnaire controller is responsible for creating, displaying and managing these items. Therefore, due to the importance of this controller, refactoring it was of interest to help improve the readability of its features.

  • Replaced literal values with defined constants to aid in the understanding of code behavior.
  • Investigated whether the method create_questionnaire is used.
  • Fixed the methods that have a questionnaire_id as a parameter, as it is not needed, since those methods should be contained within the model class anyways.
  • Removed one of three checks for QuizQuestionnaire.
  • Corrected the dropdown's default alternatives to reflect the questionnaire's min and max question score.
  • Updated the dropdown's RSpec test to reflect the change in behavior.
  • Discovered more software faults.
  • Moved three of four private methods from the controller to the model.

Results for investigating create_questionnaire

The reason for this method is when there is a teaching assistant that is creating a questionnaire for an instructor, the questionnaire's instructor_id field is assigned to be the instructor of the course, instead of the user's (in this case, the TA). The RSpec test for this method expects that the application will be redirected to tree_display after calling this method, therefore it is unclear if the application could instead redirect to questionnaires/edit, or if it has to redirect to tree_display. The normal create function assigns the questionnaire's instructor_id as the id of the current user, and redirects to questionnaires/edit. However, we have not seen an instance where create_questionnaire is used, except in the RSpec test. Thus, it may be possible to move some of the code from create_questionnaire into the create method, if the RSpec test can be safely deleted. Though, we do not know if that is the case.

Methods that had questionnaire_id as a parameter

Some methods in the Questionnaire controller had a parameter called questionnaire_id, and it was suspected that we could use params[:id] instead. However, we found that those methods were also private and should instead be in the model class. By moving those methods to the model class, we were able to remove the questionnaire_id parameter, and in some cases replace it with a params argument instead.

Checking for QuizQuestionnaire

The Questionnaire controller checks to see if the questionnaire type is QuizQuestionnaire, and it is believed that this may be unnecessary. We were able to eliminate one of three checks in the code, namely, the method save_new_questions which has been moved to the model class.

Dropdown default alternatives

Before our refactoring, the dropdown had a default literal value of '0|1|2|3|4|5' which was an error. This is an error because it is possible it exceeded the default value of the questionnaire max question score or allowed for points below the questionnaire min question score. Instead, this was replaced with code to make the dropdown's alternatives more responsive to the Questionnaire's min/max question score. However, we noticed that when the Questionnaire's min/max question score is edited after it has been created, the dropdown's alternatives could violate the new constraints, and also upon adding a new item e.g. dropdown, the edited (but not saved) min/max question score is reset to the initial values that were set upon creation of the Questionnaire.

Dropdown's RSpec test

Upon fixing the dropdown's behavior, the RSpec test did not meet the expected behavior since before the alternatives were hardcoded in. So, now the dropdown code requires the min/max question score to be set in the questionnaire it is assigned to, so that the default value for the dropdown's alternatives do not violate the constraints of the questionnaire.

Discovered more software faults

See Dropdown default alternatives.

Extract private methods from controller to model

Three of the four private methods that were in the controller were able to be removed and placed within the Questionnaire model class. The remaining private method called save was left in the controller since RSpec tests expected to find it in that location, and it appeared to be tightly coupled with existing functionality.