Expertiza_Wiki - User contributions [en]

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

2023-12-06T22:05:12Z

Acbondi: Refactored the documentation to be more easily read

=='''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.

=='''Create and Update'''==

# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
flash[:error] = error_message
end
# If penalty is valid then tries to update and save.
if valid_penalty
@late_policy = LatePolicy.new(late_policy_params)
@late_policy.instructor_id = instructor_id
begin
@late_policy.save!
flash[:notice] = 'The late policy was successfully created.'
redirect_to action: 'index'
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
rescue StandardError
flash[:error] = 'The following error occurred while saving the late policy: '
redirect_to action: 'new'
end
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to action: 'new'
end
end

# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
penalty_policy.save!
LatePolicy.update_calculated_penalty_objects(penalty_policy)
flash[:notice] = 'The late policy was successfully updated.'
redirect_to action: 'index'
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
flash[:error] = 'The following error occurred while updating the late policy: '
redirect_to action: 'edit', id: params[:id]
end
end
end

In the create and update methods, there are multiple if-else conditions and error handling which can be refactored for better readability and maintainability. The above code snippets represent the create and update functions. Both are very long and could benefit from splitting up the duplicated code and simplification.

== Changes ==

These functions were refactored into multiple new helper functions. They were created in the hopes to reduce the amount of repeated code as well as increase the readability and maintainability of said functions.

Create: The create function has been simplified to improve its readability and size.
# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
flash[:error] = error_message if error_message
# If penalty is valid then tries to update and save.
begin
if valid_penalty
@late_policy = LatePolicy.new(params)
@late_policy.instructor_id = instructor_id
valid_penalty = save_late_policy
end
rescue StandardError
valid_penalty = false
end
# Redirect to new if there's an error, index if not
redirect_to action: (valid_penalty ? 'index' : 'new')
end

Update: The update function received a similar improvement, being shortened and simplified.
# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input(true)
if !valid_penalty
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
penalty_policy.update_attributes(late_policy_params)
error_thrown = save_late_policy(true)
# If there was an error thrown, go back to edit, otherwise go to index
if error_thrown
redirect_to action: 'edit', id: params[:id]
else
redirect_to action: 'index'
end
end
end

Both create and update have been refactored and split up into various helper functions, many of them to help reduce duplicate code and size in their caller.
# Saves the late policy called from create or update
def save_late_policy(from_update = false)
begin
@late_policy.save!
# If the method that called this is update
LatePolicy.update_calculated_penalty_objects(penalty_policy) if from_update
# The code at the end of the string gets the name of the last method (create, update) and adds a d (created, updated)
flash_for_save(from_update)
rescue StandardError
# If something unexpected happens while saving the record in to database then displays a flash notice
flash[:error] = 'The following error occurred while saving the late policy: '
return false
end
true
end

def flash_for_save(from_update = false)
flash[:notice] = "The late policy was successfully #{from_update ? 'updated' : 'created'}."
end

The save_late_policy function has been refactored from duplicate code in both create and update. Both functions had very similar code with only one line of difference as well as a few characters in strings. This function shortens both of the other functions down by separating out this duplicate code into a private function. The same can be said for both the handle_error function. The flash_for_save function also improves the cognitive complexity of the save function by taking out an if statement into another function.

=='''Validate_input'''==

# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty, error_message = duplicate_name_check(is_update)
prefix = is_update ? "Cannot edit the policy. " : ""
# This check validates the maximum penalty.
if max_penalty < penalty_per_unit
error_message = prefix + 'The maximum penalty cannot be less than penalty per unit.'
valid_penalty = false
end
# This check validates the penalty per unit for a late policy.
if penalty_per_unit < 0
error_message = 'Penalty per unit cannot be negative.'
valid_penalty = false
end
# This checks maximum penalty does not exceed 100.
if max_penalty >= 100
error_message = prefix + 'Maximum penalty cannot be greater than or equal to 100'
valid_penalty = false
end
return valid_penalty, error_message
end

The validate_input method is quite lengthy and has multiple conditions being checked. It might be worth breaking down this method into smaller functions, each handling a specific validation to make this function more readable.

== Changes ==

The validate_input function was originally very long and confusing. It has since been simplified down and separated into helper functions so each function only has one job.
# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
error_messages = []
# Validates the name is not a duplicate
valid_penalty, name_error = duplicate_name_check(is_update)
error_messages << name_error if name_error
# This validates the max_penalty to make sure it's within the correct range
if max_penalty_valid(max_penalty, penalty_per_unit)
error_messages << "#{error_prefix(is_update)}The maximum penalty must be between the penalty per unit and 100."
valid_penalty = false
end
# This validates the penalty_per_unit and makes sure it's not negative
if penalty_per_unit_valid(penalty_per_unit)
error_messages << 'Penalty per unit cannot be negative.'
valid_penalty = false
end
[valid_penalty, error_messages.join("\n")]
end

# Validate the maximum penalty and ensure it's in the correct range
def max_penalty_valid(max_penalty, penalty_per_unit)
max_penalty < penalty_per_unit || max_penalty > 100
end

# Validates the penalty per unit
def penalty_per_unit_valid(penalty_per_unit)
penalty_per_unit < 0
end

# Validation error prefix
def error_prefix(is_update)
is_update ? "Cannot edit the policy. " : ""
end

The new validate_input function has combined two of the conditions that validated the same input, max_penalty, into one helper function, max_penalty_valid. This new helper function validates everything with max_penalty. Additionally, the penalty_per_unit validation is moved to another new function. The new error_prefix function is a small helper function that obtains the prefix for specific errors, as per the original validate_input. Lastly, the original validate_input function returned the last error it found, overwriting all previous errors in favor of the last. This new validate_input returns all errors that are found in one big String.

=='''Duplicate Name Check'''==
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.

== Changes ==

The def duplicate_name_check function was originally also long not reusable according to DRY principles. It has since been simplified down and separated into helper functions so each function only has one job.
# This function checks if the policy name already exists or not and returns boolean value for penalty and the error message.
def duplicate_name_check(is_update = false)
valid_penalty, error_message = true, nil
prefix = is_update ? "Cannot edit the policy. " : ""

if should_check_policy_name?(is_update)
valid_penalty, error_message = check_for_duplicate_name(prefix)
end

return valid_penalty, error_message
end

# This is a helper function for the duplicate name check
def should_check_policy_name?(is_update)
# If the function is called in the context of updating a policy
if is_update
# Find the existing late policy by its ID
existing_late_policy = LatePolicy.find(params[:id])
# Check if the policy name in the request is different from the existing policy's name
# Return true if they are different, indicating a need to check the policy name
return existing_late_policy.policy_name != params[:late_policy][:policy_name]
end
return true
end

# This is a helper function for the duplicate name check
def check_for_duplicate_name(prefix)
# Using `exists?` to check if a LatePolicy with the same name already exists for the instructor.
# It's assumed that `params[:late_policy][:policy_name]` holds the name of the policy and
# `instructor_id` is the ID of the instructor.
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
else
# If no duplicate name is found, set valid_penalty to true
valid_penalty = true
error_message = nil
end

# Return the validity status of the penalty and the corresponding error message, if any
return valid_penalty, error_message
end

The new duplicate_name_check function has been simplified down to using two helper functions to simplify the functionality of the code. It uses the should_check_policy_name and check_for_duplicate_name to make sure that there are no duplicate late policies. Each helper function has its own functionality, the first one, should_check_policy_name makes sure we need to check if there is a duplicate. The second one, check_for_duplicate_name is only used if we do need to check the duplicate and it returns whether it is valid and the error message.

=='''Proposed Solution'''==

There are multiple routes we can go down to tackle this issue. One way to break down a long method would be to refactor the create method, we can extract and rewrite the save_late_policy method. We also have to refactor the duplicate name check method by rewriting the if statements as smaller sub methods. We would then have to write tests for all the new functionality/refactoring done.

== Test Changes ==

Various duplicate code in the latest_policies_controller_spec file has been factored out into helper functions.

def create_late_policy(policy_name, max_penalty, penalty_per_unit, instructor_id)
late_policy = LatePolicy.new
late_policy.policy_name = policy_name
late_policy.max_penalty = max_penalty
late_policy.penalty_per_unit = penalty_per_unit
late_policy.instructor_id = instructor_id
late_policy
end

def request_params(policy_name, max_penalty, penalty_per_unit) {
late_policy: {
max_penalty: max_penalty,
penalty_per_unit: penalty_per_unit,
policy_name: policy_name
}
}
end

def request_params_with_id(policy_name, max_penalty, penalty_per_unit, id) {
late_policy: {
max_penalty: max_penalty,
penalty_per_unit: penalty_per_unit,
policy_name: policy_name
},
id: id
}
end

# Create an RSpec example to reduce code duplication
RSpec.shared_examples 'late policy creation with error' do |policy_name, max_penalty, penalty_per_unit, expected_error|
before(:each) do
latePolicy = LatePolicy.new
allow(latePolicy).to receive(:check_policy_with_same_name).with(any_args).and_return(false)
end
it 'throws a flash error' do
post :create, params: request_params(policy_name, max_penalty, penalty_per_unit)
expect(flash[:error]).to eq(expected_error)
expect(response).to redirect_to('/late_policies/new')
end
end

These additions to the test file, late_policies_controller_spec.rb, helps reduce duplicate code in the test by refactoring the shared components into these helper methods. Each helper method sets up specific late_policies to be used in various tests in the file.

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

2023-12-05T05:06:03Z

Acbondi: Updated the wiki with the new changes to the code

=='''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.

== Create and Update Functions ==

# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
flash[:error] = error_message
end
# If penalty is valid then tries to update and save.
if valid_penalty
@late_policy = LatePolicy.new(late_policy_params)
@late_policy.instructor_id = instructor_id
begin
@late_policy.save!
flash[:notice] = 'The late policy was successfully created.'
redirect_to action: 'index'
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
rescue StandardError
flash[:error] = 'The following error occurred while saving the late policy: '
redirect_to action: 'new'
end
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to action: 'new'
end
end

# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
penalty_policy.save!
LatePolicy.update_calculated_penalty_objects(penalty_policy)
flash[:notice] = 'The late policy was successfully updated.'
redirect_to action: 'index'
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
flash[:error] = 'The following error occurred while updating the late policy: '
redirect_to action: 'edit', id: params[:id]
end
end
end

In the create and update methods, there are multiple if-else conditions and error handling which can be refactored for better readability and maintainability. The above code snippets represent the create and update functions. Both are very long and could benefit from splitting up the duplicated code and simplification.

== Validate_input Function ==

# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty, error_message = duplicate_name_check(is_update)
prefix = is_update ? "Cannot edit the policy. " : ""
# This check validates the maximum penalty.
if max_penalty < penalty_per_unit
error_message = prefix + 'The maximum penalty cannot be less than penalty per unit.'
valid_penalty = false
end
# This check validates the penalty per unit for a late policy.
if penalty_per_unit < 0
error_message = 'Penalty per unit cannot be negative.'
valid_penalty = false
end
# This checks maximum penalty does not exceed 100.
if max_penalty >= 100
error_message = prefix + 'Maximum penalty cannot be greater than or equal to 100'
valid_penalty = false
end
return valid_penalty, error_message
end

The validate_input method is quite lengthy and has multiple conditions being checked. It might be worth breaking down this method into smaller functions, each handling a specific validation to make this function more readable.

== 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.

=='''Proposed Solution'''==

There are multiple routes we can go down to tackle this issue. One way to break down a long method would be to refactor the create method, we can extract and rewrite the save_late_policy method. We also have to refactor the duplicate name check method by rewriting the if statements as smaller sub methods. We would then have to write tests for all the new functionality/refactoring done.

=='''Updated Works'''==

== Create and Update ==

The Create and Update functions were refactored into multiple new helper functions. These functions were created in the hopes to reduce the amount of repeated code as well as increase the readability and maintainability of said functions.

Create: The create function has been simplified to improve its readability and size.
# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
flash[:error] = error_message if error_message
# If penalty is valid then tries to update and save.
begin
if valid_penalty
@late_policy = LatePolicy.new(params)
@late_policy.instructor_id = instructor_id
valid_penalty = save_late_policy
end
rescue StandardError
valid_penalty = false
end
# Redirect to new if there's an error, index if not
redirect_to action: (valid_penalty ? 'index' : 'new')
end

Update: The update function received a similar improvement, being shortened and simplified.
# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input(true)
if !valid_penalty
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
penalty_policy.update_attributes(late_policy_params)
error_thrown = save_late_policy(true)
# If there was an error thrown, go back to edit, otherwise go to index
if error_thrown
redirect_to action: 'edit', id: params[:id]
else
redirect_to action: 'index'
end
end
end

Both create and update have been refactored and split up into various helper functions, many of them to help reduce duplicate code and size in their caller.
# Saves the late policy called from create or update
def save_late_policy(from_update = false)
begin
@late_policy.save!
# If the method that called this is update
LatePolicy.update_calculated_penalty_objects(penalty_policy) if from_update
# The code at the end of the string gets the name of the last method (create, update) and adds a d (created, updated)
flash_for_save(from_update)
rescue StandardError
# If something unexpected happens while saving the record in to database then displays a flash notice
flash[:error] = 'The following error occurred while saving the late policy: '
return false
end
true
end

def flash_for_save(from_update = false)
flash[:notice] = "The late policy was successfully #{from_update ? 'updated' : 'created'}."
end

The save_late_policy function has been refactored from duplicate code in both create and update. Both functions had very similar code with only one line of difference as well as a few characters in strings. This function shortens both of the other functions down by separating out this duplicate code into a private function. The same can be said for both the handle_error function. The flash_for_save function also improves the cognitive complexity of the save function by taking out an if statement into another function.

== Validate_input ==

The validate_input function was originally very long and confusing. It has since been simplified down and separated into helper functions so each function only has one job.
# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
error_messages = []
# Validates the name is not a duplicate
valid_penalty, name_error = duplicate_name_check(is_update)
error_messages << name_error if name_error
# This validates the max_penalty to make sure it's within the correct range
if max_penalty_validation(max_penalty, penalty_per_unit)
error_messages << "#{error_prefix(is_update)}The maximum penalty must be between the penalty per unit and 100."
valid_penalty = false
end
# This validates the penalty_per_unit and makes sure it's not negative
if penalty_per_unit_validation(penalty_per_unit)
error_messages << 'Penalty per unit cannot be negative.'
valid_penalty = false
end
[valid_penalty, error_messages.join("\n")]
end

# Validate the maximum penalty and ensure it's in the correct range
def max_penalty_validation(max_penalty, penalty_per_unit)
max_penalty < penalty_per_unit || max_penalty > 100
end

# Validates the penalty per unit
def penalty_per_unit_validation(penalty_per_unit)
penalty_per_unit < 0
end

# Validation error prefix
def error_prefix(is_update)
is_update ? "Cannot edit the policy. " : ""
end

The new validate_input function has combined two of the conditions that validated the same input, max_penalty, into one helper function, max_penalty_validation. This new helper function validates everything with max_penalty. Additionally, the penalty_per_unit validation is moved to another new function. The new error_prefix function is a small helper function that obtains the prefix for specific errors, as per the original validate_input. Lastly, the original validate_input function returned the last error it found, overwriting all previous errors in favor of the last. This new validate_input returns all errors that are found in one big String.

== Duplicate Name Check Function ==

The def duplicate_name_check function was originally also long not reusable according to DRY principles. It has since been simplified down and separated into helper functions so each function only has one job.
# This function checks if the policy name already exists or not and returns boolean value for penalty and the error message.
def duplicate_name_check(is_update = false)
valid_penalty, error_message = true, nil
prefix = is_update ? "Cannot edit the policy. " : ""

if should_check_policy_name?(is_update)
valid_penalty, error_message = check_for_duplicate_name(prefix)
end

return valid_penalty, error_message
end

# This is a helper function for the duplicate name check
def should_check_policy_name?(is_update)
# If the function is called in the context of updating a policy
if is_update
# Find the existing late policy by its ID
existing_late_policy = LatePolicy.find(params[:id])
# Check if the policy name in the request is different from the existing policy's name
# Return true if they are different, indicating a need to check the policy name
return existing_late_policy.policy_name != params[:late_policy][:policy_name]
end
return true
end

# This is a helper function for the duplicate name check
def check_for_duplicate_name(prefix)
# Using `exists?` to check if a LatePolicy with the same name already exists for the instructor.
# It's assumed that `params[:late_policy][:policy_name]` holds the name of the policy and
# `instructor_id` is the ID of the instructor.
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
else
# If no duplicate name is found, set valid_penalty to true
valid_penalty = true
error_message = nil
end

# Return the validity status of the penalty and the corresponding error message, if any
return valid_penalty, error_message
end

The new duplicate_name_check function has been simplified down to using two helper functions to simplify the functionality of the code. It uses the should_check_policy_name and check_for_duplicate_name to make sure that there are no duplicate late policies. Each helper function has its own functionality, the first one, should_check_policy_name makes sure we need to check if there is a duplicate. The second one, check_for_duplicate_name is only used if we do need to check the duplicate and it returns whether it is valid and the error message.

== Test Changes ==

Various duplicate code in the latest_policies_controller_spec file has been factored out into helper functions.

def create_late_policy(policy_name, max_penalty, penalty_per_unit, instructor_id)
late_policy = LatePolicy.new
late_policy.policy_name = policy_name
late_policy.max_penalty = max_penalty
late_policy.penalty_per_unit = penalty_per_unit
late_policy.instructor_id = instructor_id
late_policy
end

def request_params(policy_name, max_penalty, penalty_per_unit) {
late_policy: {
max_penalty: max_penalty,
penalty_per_unit: penalty_per_unit,
policy_name: policy_name
}
}
end

def request_params_with_id(policy_name, max_penalty, penalty_per_unit, id) {
late_policy: {
max_penalty: max_penalty,
penalty_per_unit: penalty_per_unit,
policy_name: policy_name
},
id: id
}
end

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

2023-12-04T23:17:17Z

Acbondi: Changed the code segments in the Updated

=='''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.

== Create and Update Functions ==

# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
flash[:error] = error_message
end
# If penalty is valid then tries to update and save.
if valid_penalty
@late_policy = LatePolicy.new(late_policy_params)
@late_policy.instructor_id = instructor_id
begin
@late_policy.save!
flash[:notice] = 'The late policy was successfully created.'
redirect_to action: 'index'
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
rescue StandardError
flash[:error] = 'The following error occurred while saving the late policy: '
redirect_to action: 'new'
end
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to action: 'new'
end
end

# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
penalty_policy.save!
LatePolicy.update_calculated_penalty_objects(penalty_policy)
flash[:notice] = 'The late policy was successfully updated.'
redirect_to action: 'index'
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
flash[:error] = 'The following error occurred while updating the late policy: '
redirect_to action: 'edit', id: params[:id]
end
end
end

In the create and update methods, there are multiple if-else conditions and error handling which can be refactored for better readability and maintainability. The above code snippets represent the create and update functions. Both are very long and could benefit from splitting up the duplicated code and simplification.

== Validate_input Function ==

# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty, error_message = duplicate_name_check(is_update)
prefix = is_update ? "Cannot edit the policy. " : ""
# This check validates the maximum penalty.
if max_penalty < penalty_per_unit
error_message = prefix + 'The maximum penalty cannot be less than penalty per unit.'
valid_penalty = false
end
# This check validates the penalty per unit for a late policy.
if penalty_per_unit < 0
error_message = 'Penalty per unit cannot be negative.'
valid_penalty = false
end
# This checks maximum penalty does not exceed 100.
if max_penalty >= 100
error_message = prefix + 'Maximum penalty cannot be greater than or equal to 100'
valid_penalty = false
end
return valid_penalty, error_message
end

The validate_input method is quite lengthy and has multiple conditions being checked. It might be worth breaking down this method into smaller functions, each handling a specific validation to make this function more readable.

== 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.

=='''Proposed Solution'''==

There are multiple routes we can go down to tackle this issue. One way to break down a long method would be to refactor the create method, we can extract and rewrite the save_late_policy method. We also have to refactor the duplicate name check method by rewriting the if statements as smaller sub methods. We would then have to write tests for all the new functionality/refactoring done.

=='''Updated Works'''==

== Create and Update ==

The Create and Update functions were refactored into multiple new helper functions. These functions were created in the hopes to reduce the amount of repeated code as well as increase the readability and maintainability of said functions.

Create: The create function has been simplified to improve its readability and size.
# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
handle_error(error_message)
end
# If penalty is valid then tries to update and save.
if valid_penalty
create_new_late_policy(late_policy_params)
error_thrown = save_late_policy
if error_thrown
# If there was an error thrown go to new, otherwise go to index
redirect_to action: 'new'
else
redirect_to action: 'index'
end
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to action: 'new'
end
end

Update: The update function received a similar improvement, being shortened and simplified.
# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input(true)
if !valid_penalty
handle_error(error_message)
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
penalty_policy.update_attributes(late_policy_params)
error_thrown = save_late_policy
# If there was an error thrown, go back to edit, otherwise go to index
if error_thrown
redirect_to action: 'edit', id: params[:id]
else
redirect_to action: 'index'
end
end
end

Both create and update have been refactored and split up into various helper functions, many of them to help reduce duplicate code and size in their caller.
# Create and save the late policy with the required params
def create_new_late_policy(params)
@late_policy = LatePolicy.new(params)
@late_policy.instructor_id = instructor_id
end

# Saves the late policy called from create or update
def save_late_policy
error_thrown = false
begin
@late_policy.save!
if caller_locations(2,1)[0].label == 'update'
# If the method that called this is update
LatePolicy.update_calculated_penalty_objects(penalty_policy)
end
# The code at the end of the string gets the name of the last method (create, update) and adds a d (created, updated)
flash[:notice] = "The late policy was successfully #{caller_locations(2,1)[0].label}d."
rescue StandardError
error_thrown = true
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
if caller_locations(2,1)[0].label == 'update'
message_thrown = 'The following error occurred while updating the late policy: '
else
message_thrown = 'The following error occurred while saving the late policy: '
end
handle_error(message_thrown)
end
error_thrown
end

# A method to extrapolate out the flashing of error messages
def handle_error(error_message)
flash[:error] = error_message
end

The create_new_late_policy function is refactored from the create method to move all the code required to create a new blank late policy into one method. This slightly improves readability in the create method. The save_late_policy function has been refactored from duplicate code in both create and update. Both functions had very similar code with only one line of difference as well as a few characters in strings. This function shortens both of the other functions down by separating out this duplicate code into a private function. The same can be said for both the handle_error function.

== Validate_input ==

The validate_input function was originally very long and confusing. It has since been simplified down and separated into helper functions so each function only has one job.
# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
error_messages = []
# Validates the name is not a duplicate
valid_penalty, name_error = duplicate_name_check(is_update)
error_messages << name_error if name_error
# This validates the max_penalty to make sure it's within the correct range
if max_penalty_validation(max_penalty, penalty_per_unit)
error_messages << "#{error_prefix(is_update)}The maximum penalty must be between the penalty per unit and 100."
valid_penalty = false
end
# This validates the penalty_per_unit and makes sure it's not negative
if penalty_per_unit_validation(penalty_per_unit)
error_messages << "Penalty per unit cannot be negative."
valid_penalty = false
end
[valid_penalty, error_messages.join("\n")]
end

# Validate the maximum penalty and ensure it's in the correct range
def max_penalty_validation(max_penalty, penalty_per_unit)
max_penalty < penalty_per_unit || max_penalty > 100
end

# Validates the penalty per unit
def penalty_per_unit_validation(penalty_per_unit)
penalty_per_unit < 0
end

# Validation error prefix
def error_prefix(is_update)
is_update ? "Cannot edit the policy. " : ""
end

The new validate_input function has combined two of the conditions that validated the same input, max_penalty, into one helper function, max_penalty_validation. This new helper function validates everything with max_penalty. Additionally, the penalty_per_unit validation is moved to another new function. The new error_prefix function is a small helper function that obtains the prefix for specific errors, as per the original validate_input. Lastly, the original validate_input function returned the last error it found, overwriting all previous errors in favor of the last. This new validate_input returns all errors that are found in one big String.

== Test Changes ==

Various duplicate code in the latest_policies_controller_spec file has been factored out into helper functions.

def create_late_policy(policy_name, max_penalty, penalty_per_unit, instructor_id)
late_policy = LatePolicy.new
late_policy.policy_name = policy_name
late_policy.max_penalty = max_penalty
late_policy.penalty_per_unit = penalty_per_unit
late_policy.instructor_id = instructor_id
late_policy
end

def request_params(policy_name, max_penalty, penalty_per_unit) {
late_policy: {
max_penalty: max_penalty,
penalty_per_unit: penalty_per_unit,
policy_name: policy_name
}
}
end

def request_params_with_id(policy_name, max_penalty, penalty_per_unit, id) {
late_policy: {
max_penalty: max_penalty,
penalty_per_unit: penalty_per_unit,
policy_name: policy_name
},
id: id
}
end

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

2023-12-04T20:30:54Z

Acbondi: Fixed a typo in the test changes code

=='''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.

== Create and Update Functions ==

# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
flash[:error] = error_message
end
# If penalty is valid then tries to update and save.
if valid_penalty
@late_policy = LatePolicy.new(late_policy_params)
@late_policy.instructor_id = instructor_id
begin
@late_policy.save!
flash[:notice] = 'The late policy was successfully created.'
redirect_to action: 'index'
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
rescue StandardError
flash[:error] = 'The following error occurred while saving the late policy: '
redirect_to action: 'new'
end
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to action: 'new'
end
end

# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
penalty_policy.save!
LatePolicy.update_calculated_penalty_objects(penalty_policy)
flash[:notice] = 'The late policy was successfully updated.'
redirect_to action: 'index'
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
flash[:error] = 'The following error occurred while updating the late policy: '
redirect_to action: 'edit', id: params[:id]
end
end
end

In the create and update methods, there are multiple if-else conditions and error handling which can be refactored for better readability and maintainability. The above code snippets represent the create and update functions. Both are very long and could benefit from splitting up the duplicated code and simplification.

== Validate_input Function ==

# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty, error_message = duplicate_name_check(is_update)
prefix = is_update ? "Cannot edit the policy. " : ""
# This check validates the maximum penalty.
if max_penalty < penalty_per_unit
error_message = prefix + 'The maximum penalty cannot be less than penalty per unit.'
valid_penalty = false
end
# This check validates the penalty per unit for a late policy.
if penalty_per_unit < 0
error_message = 'Penalty per unit cannot be negative.'
valid_penalty = false
end
# This checks maximum penalty does not exceed 100.
if max_penalty >= 100
error_message = prefix + 'Maximum penalty cannot be greater than or equal to 100'
valid_penalty = false
end
return valid_penalty, error_message
end

The validate_input method is quite lengthy and has multiple conditions being checked. It might be worth breaking down this method into smaller functions, each handling a specific validation to make this function more readable.

== 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.

=='''Proposed Solution'''==

There are multiple routes we can go down to tackle this issue. One way to break down a long method would be to refactor the create method, we can extract and rewrite the save_late_policy method. We also have to refactor the duplicate name check method by rewriting the if statements as smaller sub methods. We would then have to write tests for all the new functionality/refactoring done.

=='''Updated Works'''==

== Create and Update ==

The Create and Update functions were refactored into multiple new helper functions. These functions were created in the hopes to reduce the amount of repeated code as well as increase the readability and maintainability of said functions.

Create: The create function has been simplified to improve its readability and size.
# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
handle_error(error_message)
redirect_to_policy('new')
end
# If penalty is valid then tries to update and save.
if valid_penalty
create_new_late_policy(late_policy_params)
save_late_policy
redirect_to_policy('index')
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to_policy('new')
end
end

Update: The update function received a similar improvement, being shortened and simplified.
# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
handle_error(error_message)
redirect_to_policy('edit')
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
save_late_policy
redirect_to_policy('index')
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
handle_error('The following error occurred while updating the late policy: ')
redirect_to_policy('edit')
end
end
end

Both create and update have been refactored and split up into various helper functions, many of them to help reduce duplicate code and size in their caller.
# Create and save the late policy with the required params
def create_new_late_policy(params)
@late_policy = LatePolicy.new(params)
@late_policy.instructor_id = instructor_id
end

# Saves the late policy called from create or update
def save_late_policy
begin
@late_policy.save!
if caller_locations(2,1)[0].label == 'update'
# If the method that called this is update
LatePolicy.update_calculated_penalty_objects(penalty_policy)
end
# The code at the end of the string gets the name of the last method (create, update) and adds a d (created, updated)
flash[:notice] = "The late policy was successfully #{caller_locations(2,1)[0].label}d."
rescue StandardError
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
handle_error('The following error occurred while saving the late policy: ')
redirect_to_policy('new')
end
end

# A method to extrapolate out the flashing of error messages
def handle_error(error_message)
flash[:error] = error_message
end

# A method to extrapolate out the redirecting to policy controller states
def redirect_to_policy(location)
if location == "edit"
# If the location is the edit screen, use the old id that was inputted
redirect_to action: location, id: params[:id]
else
redirect_to action: location
end
end

The create_new_late_policy function is refactored from the create method to move all the code required to create a new blank late policy into one method. This slightly improves readability in the create method. The save_late_policy function has been refactored from duplicate code in both create and update. Both functions had very similar code with only one line of difference as well as a few characters in strings. This function shortens both of the other functions down by separating out this duplicate code into a private function. The same can be said for both the handle_error function and redirect_to_policy function.

== Validate_input ==

The validate_input function was originally very long and confusing. It has since been simplified down and separated into helper functions so each function only has one job.
# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty = true
error_messages = []
# Validates the name is not a duplicate
valid_penalty, name_error = duplicate_name_check(is_update)
error_messages << name_error if name_error
# This validates the max_penalty to make sure it's within the correct range
if max_penalty_validation(max_penalty, penalty_per_unit)
error_messages << "#{error_prefix(is_update)}The maximum penalty must be between the penalty per unit and 100."
valid_penalty = false
end
# This validates the penalty_per_unit and makes sure it's not negative
if penalty_per_unit_validation(penalty_per_unit)
error_messages << "Penalty per unit cannot be negative."
valid_penalty = false
end
[valid_penalty, error_messages.join("\n")]
end

# Validate the maximum penalty and ensure it's in the correct range
def max_penalty_validation(max_penalty, penalty_per_unit)
max_penalty < penalty_per_unit || max_penalty > 100
end

# Validates the penalty per unit
def penalty_per_unit_validation(penalty_per_unit)
penalty_per_unit < 0
end

# Validation error prefix
def error_prefix(is_update)
is_update ? "Cannot edit the policy. " : ""
end

The new validate_input function has combined two of the conditions that validated the same input, max_penalty, into one helper function, max_penalty_validation. This new helper function validates everything with max_penalty. Additionally, the penalty_per_unit validation is moved to another new function. The new error_prefix function is a small helper function that obtains the prefix for specific errors, as per the original validate_input. Lastly, the original validate_input function returned the last error it found, overwriting all previous errors in favor of the last. This new validate_input returns all errors that are found in one big String.

== Test Changes ==

Various duplicate code in the latest_policies_controller_spec file has been factored out into helper functions.

def create_late_policy(policy_name, max_penalty, penalty_per_unit, instructor_id)
late_policy = LatePolicy.new
late_policy.policy_name = policy_name
late_policy.max_penalty = max_penalty
late_policy.penalty_per_unit = penalty_per_unit
late_policy.instructor_id = instructor_id
late_policy
end

def request_params(policy_name, max_penalty, penalty_per_unit) {
late_policy: {
max_penalty: max_penalty,
penalty_per_unit: penalty_per_unit,
policy_name: policy_name
}
}
end

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

2023-12-04T19:59:28Z

Acbondi: Added the changes to tests section and added what I changed

=='''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.

== Create and Update Functions ==

# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
flash[:error] = error_message
end
# If penalty is valid then tries to update and save.
if valid_penalty
@late_policy = LatePolicy.new(late_policy_params)
@late_policy.instructor_id = instructor_id
begin
@late_policy.save!
flash[:notice] = 'The late policy was successfully created.'
redirect_to action: 'index'
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
rescue StandardError
flash[:error] = 'The following error occurred while saving the late policy: '
redirect_to action: 'new'
end
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to action: 'new'
end
end

# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
penalty_policy.save!
LatePolicy.update_calculated_penalty_objects(penalty_policy)
flash[:notice] = 'The late policy was successfully updated.'
redirect_to action: 'index'
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
flash[:error] = 'The following error occurred while updating the late policy: '
redirect_to action: 'edit', id: params[:id]
end
end
end

In the create and update methods, there are multiple if-else conditions and error handling which can be refactored for better readability and maintainability. The above code snippets represent the create and update functions. Both are very long and could benefit from splitting up the duplicated code and simplification.

== Validate_input Function ==

# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty, error_message = duplicate_name_check(is_update)
prefix = is_update ? "Cannot edit the policy. " : ""
# This check validates the maximum penalty.
if max_penalty < penalty_per_unit
error_message = prefix + 'The maximum penalty cannot be less than penalty per unit.'
valid_penalty = false
end
# This check validates the penalty per unit for a late policy.
if penalty_per_unit < 0
error_message = 'Penalty per unit cannot be negative.'
valid_penalty = false
end
# This checks maximum penalty does not exceed 100.
if max_penalty >= 100
error_message = prefix + 'Maximum penalty cannot be greater than or equal to 100'
valid_penalty = false
end
return valid_penalty, error_message
end

The validate_input method is quite lengthy and has multiple conditions being checked. It might be worth breaking down this method into smaller functions, each handling a specific validation to make this function more readable.

== 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.

=='''Proposed Solution'''==

There are multiple routes we can go down to tackle this issue. One way to break down a long method would be to refactor the create method, we can extract and rewrite the save_late_policy method. We also have to refactor the duplicate name check method by rewriting the if statements as smaller sub methods. We would then have to write tests for all the new functionality/refactoring done.

=='''Updated Works'''==

== Create and Update ==

The Create and Update functions were refactored into multiple new helper functions. These functions were created in the hopes to reduce the amount of repeated code as well as increase the readability and maintainability of said functions.

Create: The create function has been simplified to improve its readability and size.
# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
handle_error(error_message)
redirect_to_policy('new')
end
# If penalty is valid then tries to update and save.
if valid_penalty
create_new_late_policy(late_policy_params)
save_late_policy
redirect_to_policy('index')
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to_policy('new')
end
end

Update: The update function received a similar improvement, being shortened and simplified.
# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
handle_error(error_message)
redirect_to_policy('edit')
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
save_late_policy
redirect_to_policy('index')
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
handle_error('The following error occurred while updating the late policy: ')
redirect_to_policy('edit')
end
end
end

Both create and update have been refactored and split up into various helper functions, many of them to help reduce duplicate code and size in their caller.
# Create and save the late policy with the required params
def create_new_late_policy(params)
@late_policy = LatePolicy.new(params)
@late_policy.instructor_id = instructor_id
end

# Saves the late policy called from create or update
def save_late_policy
begin
@late_policy.save!
if caller_locations(2,1)[0].label == 'update'
# If the method that called this is update
LatePolicy.update_calculated_penalty_objects(penalty_policy)
end
# The code at the end of the string gets the name of the last method (create, update) and adds a d (created, updated)
flash[:notice] = "The late policy was successfully #{caller_locations(2,1)[0].label}d."
rescue StandardError
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
handle_error('The following error occurred while saving the late policy: ')
redirect_to_policy('new')
end
end

# A method to extrapolate out the flashing of error messages
def handle_error(error_message)
flash[:error] = error_message
end

# A method to extrapolate out the redirecting to policy controller states
def redirect_to_policy(location)
if location == "edit"
# If the location is the edit screen, use the old id that was inputted
redirect_to action: location, id: params[:id]
else
redirect_to action: location
end
end

The create_new_late_policy function is refactored from the create method to move all the code required to create a new blank late policy into one method. This slightly improves readability in the create method. The save_late_policy function has been refactored from duplicate code in both create and update. Both functions had very similar code with only one line of difference as well as a few characters in strings. This function shortens both of the other functions down by separating out this duplicate code into a private function. The same can be said for both the handle_error function and redirect_to_policy function.

== Validate_input ==

The validate_input function was originally very long and confusing. It has since been simplified down and separated into helper functions so each function only has one job.
# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty = true
error_messages = []
# Validates the name is not a duplicate
valid_penalty, name_error = duplicate_name_check(is_update)
error_messages << name_error if name_error
# This validates the max_penalty to make sure it's within the correct range
if max_penalty_validation(max_penalty, penalty_per_unit)
error_messages << "#{error_prefix(is_update)}The maximum penalty must be between the penalty per unit and 100."
valid_penalty = false
end
# This validates the penalty_per_unit and makes sure it's not negative
if penalty_per_unit_validation(penalty_per_unit)
error_messages << "Penalty per unit cannot be negative."
valid_penalty = false
end
[valid_penalty, error_messages.join("\n")]
end

# Validate the maximum penalty and ensure it's in the correct range
def max_penalty_validation(max_penalty, penalty_per_unit)
max_penalty < penalty_per_unit || max_penalty > 100
end

# Validates the penalty per unit
def penalty_per_unit_validation(penalty_per_unit)
penalty_per_unit < 0
end

# Validation error prefix
def error_prefix(is_update)
is_update ? "Cannot edit the policy. " : ""
end

The new validate_input function has combined two of the conditions that validated the same input, max_penalty, into one helper function, max_penalty_validation. This new helper function validates everything with max_penalty. Additionally, the penalty_per_unit validation is moved to another new function. The new error_prefix function is a small helper function that obtains the prefix for specific errors, as per the original validate_input. Lastly, the original validate_input function returned the last error it found, overwriting all previous errors in favor of the last. This new validate_input returns all errors that are found in one big String.

== Test Changes ==

Various duplicate code in the latest_policies_controller_spec file has been factored out into helper functions.

def create_late_policy(policy_name, max_penalty, penalty_per_unit, instructor_id)
late_policy = LatePolicy.new
late_policy.policy_name = policy_name
late_policy.max_penalty = max_penalty
late_policy.penalty_per_unit = penalty_per_unit
late_policy.instructor_id = instructor_id
late_policy
end

def request_params(policy_name, max_penalty, penalty_per_unit) {
late_policy: {
max_penalty: max_penalty
penalty_per_unit: penalty_per_unit
policy_name: policy_name
}
}
end

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

2023-12-04T19:56:09Z

Acbondi: Added the changes to validate_input

=='''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.

== Create and Update Functions ==

# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
flash[:error] = error_message
end
# If penalty is valid then tries to update and save.
if valid_penalty
@late_policy = LatePolicy.new(late_policy_params)
@late_policy.instructor_id = instructor_id
begin
@late_policy.save!
flash[:notice] = 'The late policy was successfully created.'
redirect_to action: 'index'
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
rescue StandardError
flash[:error] = 'The following error occurred while saving the late policy: '
redirect_to action: 'new'
end
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to action: 'new'
end
end

# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
flash[:error] = error_message
redirect_to action: 'edit', id: params[:id]
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
penalty_policy.save!
LatePolicy.update_calculated_penalty_objects(penalty_policy)
flash[:notice] = 'The late policy was successfully updated.'
redirect_to action: 'index'
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
flash[:error] = 'The following error occurred while updating the late policy: '
redirect_to action: 'edit', id: params[:id]
end
end
end

In the create and update methods, there are multiple if-else conditions and error handling which can be refactored for better readability and maintainability. The above code snippets represent the create and update functions. Both are very long and could benefit from splitting up the duplicated code and simplification.

== Validate_input Function ==

# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty, error_message = duplicate_name_check(is_update)
prefix = is_update ? "Cannot edit the policy. " : ""
# This check validates the maximum penalty.
if max_penalty < penalty_per_unit
error_message = prefix + 'The maximum penalty cannot be less than penalty per unit.'
valid_penalty = false
end
# This check validates the penalty per unit for a late policy.
if penalty_per_unit < 0
error_message = 'Penalty per unit cannot be negative.'
valid_penalty = false
end
# This checks maximum penalty does not exceed 100.
if max_penalty >= 100
error_message = prefix + 'Maximum penalty cannot be greater than or equal to 100'
valid_penalty = false
end
return valid_penalty, error_message
end

The validate_input method is quite lengthy and has multiple conditions being checked. It might be worth breaking down this method into smaller functions, each handling a specific validation to make this function more readable.

== 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.

=='''Proposed Solution'''==

There are multiple routes we can go down to tackle this issue. One way to break down a long method would be to refactor the create method, we can extract and rewrite the save_late_policy method. We also have to refactor the duplicate name check method by rewriting the if statements as smaller sub methods. We would then have to write tests for all the new functionality/refactoring done.

=='''Updated Works'''==

== Create and Update Functions ==

The Create and Update functions were refactored into multiple new helper functions. These functions were created in the hopes to reduce the amount of repeated code as well as increase the readability and maintainability of said functions.

Create: The create function has been simplified to improve its readability and size.
# Create method can create a new late policy.
# There are few check points before creating a late policy which are written in the if/else statements.
def create
# First this function validates the input then save if the input is valid.
valid_penalty, error_message = validate_input
if error_message
handle_error(error_message)
redirect_to_policy('new')
end
# If penalty is valid then tries to update and save.
if valid_penalty
create_new_late_policy(late_policy_params)
save_late_policy
redirect_to_policy('index')
# If any of above checks fails, then redirect to create a new late policy again.
else
redirect_to_policy('new')
end
end

Update: The update function received a similar improvement, being shortened and simplified.
# Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
def update
penalty_policy = LatePolicy.find(params[:id])
# First this function validates the input then save if the input is valid.
_valid_penalty, error_message = validate_input(true)
if error_message
handle_error(error_message)
redirect_to_policy('edit')
# If there are no errors, then save the record.
else
begin
penalty_policy.update_attributes(late_policy_params)
save_late_policy
redirect_to_policy('index')
# If something unexpected happens while updating, then redirect to the edit page of that policy again.
rescue StandardError
handle_error('The following error occurred while updating the late policy: ')
redirect_to_policy('edit')
end
end
end

Both create and update have been refactored and split up into various helper functions, many of them to help reduce duplicate code and size in their caller.
# Create and save the late policy with the required params
def create_new_late_policy(params)
@late_policy = LatePolicy.new(params)
@late_policy.instructor_id = instructor_id
end

# Saves the late policy called from create or update
def save_late_policy
begin
@late_policy.save!
if caller_locations(2,1)[0].label == 'update'
# If the method that called this is update
LatePolicy.update_calculated_penalty_objects(penalty_policy)
end
# The code at the end of the string gets the name of the last method (create, update) and adds a d (created, updated)
flash[:notice] = "The late policy was successfully #{caller_locations(2,1)[0].label}d."
rescue StandardError
# If something unexpected happens while saving the record in to database then displays a flash notice and redirect to create a new late policy again.
handle_error('The following error occurred while saving the late policy: ')
redirect_to_policy('new')
end
end

# A method to extrapolate out the flashing of error messages
def handle_error(error_message)
flash[:error] = error_message
end

# A method to extrapolate out the redirecting to policy controller states
def redirect_to_policy(location)
if location == "edit"
# If the location is the edit screen, use the old id that was inputted
redirect_to action: location, id: params[:id]
else
redirect_to action: location
end
end

The create_new_late_policy function is refactored from the create method to move all the code required to create a new blank late policy into one method. This slightly improves readability in the create method. The save_late_policy function has been refactored from duplicate code in both create and update. Both functions had very similar code with only one line of difference as well as a few characters in strings. This function shortens both of the other functions down by separating out this duplicate code into a private function. The same can be said for both the handle_error function and redirect_to_policy function.

== Validate_input Function ==

The validate_input function was originally very long and confusing. It has since been simplified down and separated into helper functions so each function only has one job.
# This function validates the input.
def validate_input(is_update = false)
# Validates input for create and update forms
max_penalty = params[:late_policy][:max_penalty].to_i
penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
valid_penalty = true
error_messages = []
# Validates the name is not a duplicate
valid_penalty, name_error = duplicate_name_check(is_update)
error_messages << name_error if name_error
# This validates the max_penalty to make sure it's within the correct range
if max_penalty_validation(max_penalty, penalty_per_unit)
error_messages << "#{error_prefix(is_update)}The maximum penalty must be between the penalty per unit and 100."
valid_penalty = false
end
# This validates the penalty_per_unit and makes sure it's not negative
if penalty_per_unit_validation(penalty_per_unit)
error_messages << "Penalty per unit cannot be negative."
valid_penalty = false
end
[valid_penalty, error_messages.join("\n")]
end

# Validate the maximum penalty and ensure it's in the correct range
def max_penalty_validation(max_penalty, penalty_per_unit)
max_penalty < penalty_per_unit || max_penalty > 100
end

# Validates the penalty per unit
def penalty_per_unit_validation(penalty_per_unit)
penalty_per_unit < 0
end

# Validation error prefix
def error_prefix(is_update)
is_update ? "Cannot edit the policy. " : ""
end

The new validate_input function has combined two of the conditions that validated the same input, max_penalty, into one helper function, max_penalty_validation. This new helper function validates everything with max_penalty. Additionally, the penalty_per_unit validation is moved to another new function. The new error_prefix function is a small helper function that obtains the prefix for specific errors, as per the original validate_input. Lastly, the original validate_input function returned the last error it found, overwriting all previous errors in favor of the last. This new validate_input returns all errors that are found in one big String.

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

2023-12-04T19:43:36Z

Acbondi: Reworded some previous create and update solutions as well as added the changes to those functions in a new Updated Works section

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

2023-11-14T21:38:26Z

Acbondi: Moved the previous text below statement to be more clear

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

2023-11-14T21:37:27Z

Acbondi: Created more sections and filled some of them in for the page