CSC/ECE 517 Fall 2023 - E2382. Optimizing the LatePoliciesController

Introduction

The late_policies_controller.rb class houses the LatePoliciesController that controls the CRUD operations on late policies. However, there are many problems with the current implementation of this controller. In its current state, many functions are too long and repetitive as well as having inadequate variable names, comments, and error messages. This controller would benefit with optimizing its functions and various other aspects of the file.

About the LatePoliciesController

The LatePoliciesController provides CRUD functions to create, read, update, and destroy late policies. These include the index, show, new, edit, create, update, and destroy functions. Other functions are provided to allow it to work seamlessly within the framework of the overall project, including the action_allowed?, duplicate_name_check, validate_input, and various parameter and input functions. These additional functions are helper functions that ensure that the late policy can be created or updated based on if the user has the required permissions, doesn't enter a duplicate name, and inputs valid information to the late policy.

Requirements

• Refactor Long Methods: Longer functions should be refactored into smaller sub-functions to improve readability and make it easier for future alterations of the code.
• Improve Comments: More comments should be added to allow for users to easily follow through a given function and understand the specifics of its code statements.
• Follow the DRY Principle: Repeated code should be removed or moved into helper functions to allow for reusability of common code.
• Improve Testing: More tests should be created to ensure that everything works as intended and no unexpected errors occur, either exceptions or errors in logic.
• All changes must be done without the addition of new gems and must be clearly documented.

Functions to Optimize

• create: This function will be broken down into smaller functions to allow a more readable creation of new late policies. The error handling will also be altered to improve readability.
• update: Various comments will be added to the function as well as breaking down the code used for saving the late policy into a helper method to shorten the update method and make it more intuitive.
• duplicate_name_check: This function has various separate if statements that check for various things. These if statements will be broken down into separate helper methods to check for each individually. The duplicate_name_check function will be the main function that calls the various sub-functions so that it is clear what is being checked at a given step.
• validate_input: Similar to the duplicate_name_check function, this function has various if statements that can be refactored into smaller functions to check each input individually.
• Tests: Tests for creating and updating new late policies will be created. These tests will check for invalid inputs, correct error messages, etc. as well as ensure that edge cases are also captured correctly by the controller. New tests will also be created to ensure that previous functions, like the read and destroy functions, work correctly.

Duplicate Name Check Function

if is_update
     existing_late_policy = LatePolicy.find(params[:id])
     if existing_late_policy.policy_name == params[:late_policy][:policy_name]
       should_check = false
     end
   end

   if should_check
     if LatePolicy.check_policy_with_same_name(params[:late_policy][:policy_name], instructor_id)
       error_message = prefix + 'A policy with the same name ' + params[:late_policy][:policy_name] + ' already exists.'
       valid_penalty = false
     end
   end

The above code is the current implementation of some duplication checks in this function. These if statements can be refactored into smaller sub-functions. The pseudo-code for one sub-function is as follows:

if should_check_policy_name
  Call sub-function should_check_policy_name
end

def should_check_policy_name(old valid_penalty, old error_message)
  if check policy with same name(currenty name, instructor_id)
    error_message = new error message
    valid_penalty = false
  end
  return valid_penalty, error_message

In the above sub-function, the should_check name was changed to be more explicit and the should check if-statement was refactored to be in a different method to improve readability.

Test Plan

---

(Previous text below)

Problem Statement

After an instructor gave a grade to an assignment, there is no way to track who gave the grade. A grading audit trail must be created and the following information needs to be stored:

1. When a grade is assigned by an instructor, there needs to be an indication of who did it and when it was done.

2. Comments previously provided by other instructors must also be preserved.

This information needs to be stored every time an instructor edits a grade/comment and clicks the save button.

Overview of Major Changes By Previous Teams

• A new table was added to the database (grading_history), along with the corresponding model (grading_history.rb) and controller (grading_history_controller.rb).
  • Whenever an instructor submits a new grade, or edits an existing grade, the grading_history_controller saves a new history entry to the database.

• Two models for specific types of histories were added: review_grading_history.rb, and submission_grading_history.rb.

• A view for displaying the grading history of a particular assignment or review was added (grading_histories/index.html.erb).

• THIS IS FROM THE FIRST TEAM NEED TO ADD MORE FROM THE SECOND TEAM