CSC/ECE 517 Spring 2020 / E2001 Refactor Questionnaires Controller

E2000 Refactor questionnaires_controller.rb

This page provides a description of the Expertiza based OSS project

About Expertiza

Expertiza is an open source software project created using Ruby on Rails. Expertiza allows instructors to craft new assignments and edit existing ones. This flexibility ensures that each round of students gets an experience that is appropriate to the given situation. It also allows the instructor to create a list of project options and have the students bid for their favorite project.

While their are a plethora of benefits for instructors, students also gain some benefits when using Expertiza. They are able to form teams and keep track of the past peers they have worked with, and are also able to manage the progress and submission of their assignments.

Problem Statement

Background: In Expertiza, Questionnaire is a super-class utilized by all questionnaires, including rubrics, quizzes, and surveys. Rubrics are used to assess things such as project submissions and project teammates. All of the above-mentioned questionnaires are sub-classes of the Questionnaire super-class. Since this super-class is used in a multitude of locations ensuring that there are no issues in the code is very important since an error can cause malfunctions throughout Expertiza.

Problem: Questionnaires controller has been refactored repeatedly over the past few years, yet some improvements can still be made through refactoring to increase the quality and dryness of the code.

These problems are as follows:

• Assess if the method called create_questionnaire is used and remove it if not
• The create method is 49 lines long and needs to be broken up into smaller self-documenting methods
• There are three checks for whether a questionnaire is a QuizQuestionnaire, evaluate the necessity of these and remove as needed.
• Several methods have a questionnaire_id as a parameter, however This may not be necessary as params[:id] should be enough. Evaluate options and implement a solution.

---

• Unprotected mass assignment in 3 different areas
• Unsafe reflection method const_get called with parameter value in 4 locations
• Hardwired variables in add_new_questions and save_new_questions.
• Unnecessary checks for QuizQuestionnaire, checks can be removed.
• Use guard clause to enclose methods instead of conditional.

Files Modified in Project

1. app/controllers/questionnaires_controller.rb
2. spec/controllers/questionnaires_controller_spec.rb

Issues and Improvements

Use guard clause

Problem

Method(s) save_new_questions, delete, and save_questions used conditional to wrap code instead of guard clause. Using guard clause can reduce complexity and number of lines in code.

Solution

For example, we go from:

def save_new_questions(questionnaire_id)
    if params[:new_question]

To:

 def save_new_questions(questionnaire_id)
    return unless params[:new_question]

QuizQuestionnaire checks

Problem

Method(s) create and save_new_questions have unnecessary checks for if question type is QuizQuestionnaire.

Solution

We removed the check in both methods. We go from:

if @questionnaire.type == "QuizQuestionnaire"
 q.weight = 1 # setting the weight to 1 for quiz questionnaire since the model validates this field
end

To:

q.weight = 1

Hardwired Variables

Problem

Method(s) save_new_questions and add_new_questions used hardwired variables. When applicable all values used should be stored as variables so the user knows the purpose of the variable, if that value is used in multiple areas it can be changed with a single change, and it's just messy. For instance both add_new_questions and save_new_questions used the same scalar value 1 for a similar task. We go from:

if question.is_a? ScoredQuestion
 question.weight = 1
 question.max_label = 'Strongly agree'
 question.min_label = 'Strongly disagree'
end
question.size = '50, 3' if question.is_a? Criterion
question.alternatives = '0|1|2|3|4|5' if question.is_a? Dropdown
question.size = '60, 5' if question.is_a? TextArea
question.size = '30' if question.is_a? TextField

To our code which incorporates constants.

if question.is_a? ScoredQuestion
 question.weight = WEIGHT
 question.max_label = LABEL_AGREE
 question.min_label = LABEL_DISAGREE
end
question.size = SIZE_CRITERION if question.is_a? Criterion
question.alternatives = SIZE_ALT_DROPDOWN if question.is_a? Dropdown
question.size = SIZE_TXT_AREA if question.is_a? TextArea
question.size = SIZE_TXT_FIELD if question.is_a? TextField

With our constants at the top of the code for easy accessibility:

## Constants
# add_new_questions
LABEL_AGREE = 'Strongly agree' # label for scored question if agree
LABEL_DISAGREE = 'Strongly disagree' # label for scored question if disagree
WEIGHT = 1  # question weight
SIZE_CRITERION = '50, 3' # size of the question box if it's a criterion
SIZE_ALT_DROPDOWN = '0|1|2|3|4|5' # alternative to size if question is a dropdown
SIZE_TXT_AREA = '60, 5' # size of question box if text area
SIZE_TXT_FIELD = '30' # size of question box if text field

Testing Plan

In order to evaluate the changes to Expertiza throughout the OSS project two different methods were used. The first being automatic testing using RSpec and the second being manual testing through accessing the Expertiza project on one's local machine. More in depth discussion of these tests can be found below.