CSC/ECE 517 Spring 2023 - E2319. Reimplement questionnaire.rb

Problem Description

The model for questionnaire needs to be reimplemented to account for new features of recent versions of Ruby and Rails. The reimplementation of this model should consider SOLID principles and implement design patterns where relevant. The reimplementation should also consider object-oriented fundamentals like readability, eliminating redundancy, polymorphism, etc. The features available in the Ruby/Rails framework can be leveraged to accomplish these goals.

Methods Reimplemented

Each section below covers methods of the questionnaire.rb model, how they were reimplemented, and why it was done that way.

Model Validations & the “validate_questionnaire” method

In reimplementing the validation functionality of the questionnaire model, the “validate_questionnaire” method was removed entirely as it added unnecessary complexity to display errors based on if statement conditions. To replace the functionality of the “validate_questionnaire” method, active record validations using helpers were added instead. Using Active Record validations allow for better leverage of Rails’ built-in framework rather than duplicating the work the Rails is already capable of by writing conditions and queries to determine when errors should display. This also keeps the implementation simpler and easier to manage when updates in the future are needed.

There were four instances for when errors should arise to the end user from the “validate_questionnaire” method: (1) when the maximum question score entered was less than 1, (2) when the minimum question score entered was less than 0, (3) when the minimum question score entered was greater than the maximum question score, and (4) when a questionnaire is created with the name of an existing questionnaire created by the same instructor.

The first three error instances were reimplemented using the Active Record validation helper “comparison.” The first error instance, when the maximum question score entered was less than 1, was accomplished by adding a comparison to ensure that the maximum question score is greater than 0, otherwise the message “The maximum question score must be a positive integer greater than 0” will display, which is a more descriptive message than the original. The second error instance was implemented similarly to the first except for the validation was created for the minimum question score and ensured that the value is greater than or equal to 0. The third error instance was reimplemented by adding a comparison between the minimum question score and the maximum question score toe ensure that the maximum is always greater than the minimum.

Lastly, the fourth error instance of the “validate_questionnaire” method, when a questionnaire is created with the name of an existing questionnaire created by the same instructor, was reimplemented by adding the Active Record validation helper “uniqueness” to the existing name validation. In addition, the "uniqueness" helper had a scope added to ensure that uniqueness is only applied for the combination of name and instructor_id. The added uniqueness scope allows for the name to be the same on two questionnaires if created by two different instructors.

“get_weighted_score” and “compute_weighted_score” methods

These methods are used to fetch and calculate the weighted scores of all the questionnaires. While reimplementing these methods, in the "get_weighted_score" method, instead of explicitly writing the nil check condition for the "used_in_round" attribute, the safe navigation operator "&" is used. This ensures that the code doesn't break if the object is nil as well as it gets rid of the check condition for the "nil" value as it is automatically handled by the "&" operator. Also, the "if-else" condition is replaced with the ternary operator for brevity and readability. The questionnaire_symbol is now being built using the string concatenation instead of calling "to_s" on the symbol and then concatenating the result with round.to_s. This is a more concise and readable way of building the symbol.

For the "compute_weighted_score" method, while reimplementing, instead of using the array indexing way to fetch data from the variables, the "dig" method is used to retrieve the :avg score from the scores hash. The "dig" method will return nil if any of the intermediate keys are nil, so it's safe to use here as the nil condition check is done by the dig method itself instead of explicitly writing it. The use of the ternary operator is again implemented in this method as well for brevity and readability. Overall, these changes should make the code more concise, efficient, and easier to read.

“true_false_questions?” method

When reimplementing this method, the name of the method was changed to “has_true_false_questions” to be more descriptive of what a true vs false return of this method means.

This method is checking to see if there are any true/false questions associated with a questionnaire. If there are, it returns true. If there are not, the method returns false.

The original implementation of this method used the “each” method of the Array object to loop through the associated question records and return true once it came across upon a question with the type “Checkbox.” In the updated implementation of this class, instead of using “each,” the “any?” method of the Array object is used. The “any?” method automatically returns true if the criterion is met and does not need it explicitly stated within the loop like in the original implementation, allowing for the removal of unnecessary syntax.

The logic this method is intended for is more suited for the “any?” method than the “each” method. This is because the original implementation using “each” was re-writing what “any?” already does. Using “any?” also allows for better readability as it is much clearer after the reimplementation that the method will return true if any question record meets the criteria (type equals ‘Checkbox’).

“delete” method

When evaluating the "delete" method for reimplementation, it was realized that this functionality is already being done by the questionnaires controller [1]. For this reason, this method was removed in the reimplementation of this model to follow the DRY principle.

“max_possible_score” method

The “max_possible_method” method adds the values of the “weight” attribute of each question record related to the questionnaire and then multiplies that sum by the “max_question_score” of the questionnaire.

The original implementation of this method included a complex and difficult to read query which was completing all of the calculations within it.

To make this more readable and easier to update in the future several updates were made. Firstly, the reimplementation separated the summation and the multiplication operations, so that it is easier to follow how the final value is calculated and allows ease of modifications in the future. Additionally, rather than using a complex query to sum the related questions’ weight values, the “sum” method of the Array object is used instead to take advantage of Ruby’s duck typing capability and the Enumerable mixin. Using the “sum” method significantly improves readability.

The reimplementation also takes advantage of the Distributive Property in math by ensuring that the sum is calculated before applying any multiplication, rather than multiplying each weight and then summing the multiplied values. The reimplementation ensures that the calculation ‘cost effective’ in terms of CPU cycles.

“self.copy_questionnaire_details” method

The “self.copy_questionnaire_details” method clones the provided questionnaire for a given instructor. This method additionally clones question records related to the questionnaire, and question advice records for each of those questions.

To improve this in the re-implementation, more descriptive and consistent nomenclatures were used for the original and cloned objects for Questionnaire, Question and QuestionAdvice. Additionally, small improvements were made to better leverage Rails' built-in active record associations which made the code simpler and more readable.

Test Plan

The goal is to test every possible scenario. Since this is the reimplementation of a model, there are only system tests. The tests will leverage the RSpec framework and FactoryBot to stub associated records.

In order to properly stub, skeleton models of associations will be created (assignment, assignment_questionnaire, instructor, question, and questionnaire_node). Also, in order to stub successfully, tables for the associations will be created before all tests, and dropped after all tests:

Use the command below to run tests:

rspec ./spec/models/questionnaire_spec.rb

Model Validations & the “validate_questionnaire” method

Tests are to be written to ensure proper validations were in place. Some of these will use the “shoulda matchers” gem to ensure implementation of the validations as the reimplementation will be done in TDD. Additionally, behavior should be tested by ensuring the validation errors arise when expected and by ensuring valid inputs are accepted.

For the name attribute, validation tests for behavior should ensure that a questionnaire without a name cannot be saved, a questionnaire with the same name as another questionnaire cannot be saved if the instructor is the same on both questionnaires, and a questionnaire with the same name as another questionnaire can be saved if the instructors are different on each questionnaire.

Implemented Automated Tests:

For testing the validations on the min_question_score and max_question_score attributes, tests should ensure that a non-numeric value entered in either the min_question_score or max_question_score fields cannot be saved, that the min_question_score cannot be saved unless it is 0 or greater, that the max_question_score cannot be saved unless it is 1 or greater, and that the questionnaire cannot be saved if the min_question_score is greater than the max_question_score. A test should also be written to confirm that the questionnaire can save successfully when valid inputs are entered for these attributes.

Implemented Automated Tests:

“get_weighted_score” and “compute_weighted_score” methods

The test written should ensure that the correct weighted score returns depending on whether the associated assignment review has 'used_in_round' populated.

Implemented Automated Tests:

“has_true_false_questions” method

Three outcomes should tested for this method:

(1) confirming the method returns true if there is a true/false question related to the questionnaire,

(2) confirming the method returns false if there are questions related to the questionnaire but none are true/false, and

(3) confirming the method returns false if there are no questions related to the questionnaire at all.

Implemented Automated Tests:

“max_possible_score” method

Three scenarios should be tested for this method to ensure calculation is correct:

(1) Ensure that the max_possible_score returned is what is expected when there are two related questions of different weights.

(2) Ensure that the max_possible_score changes to a different expected value when a third question is added to the questionnaire.

(3) Ensure that the max_possible_score changes to a different expected value when the same three questions from the second scenario are associated but the max_question_score value is changed on the questionnaire.

Implemented Automated Tests:

“self.copy_questionnaire_details” method

Test should be written to ensure that the questionnaire is successfully cloned along with its associated question records. The test should also ensure that question advice records associated to each question record related to the questionnaire are also cloned.

Implemented Automated Tests:

Test Coverage

After implementing tests, LoC test coverage is at 82.53%:

SimpleCov Coverage Detail Report

The following SimpleCov report shows 97.96% relevant LoC for the questionnaire.rb model file: