CSC/ECE 517 Spring 2017/oss E1713
E1713. Refactor penalty_helper.rb and late_policies_controller.rb
This project is intended to improve the code quality and reduce technical debt within the Expertiza system by refactoring the penalty_helper.rb module, refactoring the late_policies_controller.rb controller, and adding test coverage for both files.
Introduction
Expertiza
Expertiza is an open source web application which allows an instructor to manage assignments. It has various features viz. creating new assignments, customizing existing assignments, automatically allocating submissions for peer review etc. It has been developed using the Ruby on Rails framework, and the code is available on Github.
Project Goals
In this project, the files late_policies_controller.rb and penalty_helper.rb were to be refactored. The methods contained in these files were too long, and needed to be either shortened, or used polymorphically, or deleted altogether if they were not being used. They also were to be refactored semantically if the code did not follow good Ruby coding practices.
Current Implementation
Late policies and penalties functionality
After an instructor creates an assignment in Expertiza they will have the ability to edit the details of the assignment by clicking the Edit action icon beside the assignment. From the "Editing Assignment" screen instructors have the ability to set due dates for each round of an assignment on the "Due dates" tab. On this tab, the instructor can elect to apply a late policy to the assignment by checking the "Apply penalty policy" checkbox and selecting a late policy from the adjacent drop down menu. If no late policies exist, or if the instructor wishes to define a new late policy, then they can click the "New late policy" link to define a new late policy.
An instructor can define a late policy with the following fields:
- Late policy name - This name will show up in the drop down menu on the "Due dates" tab when editing the assignment. It is not enforced to be unique.
- Penalty Unit - With the options of 'Minute', 'Hour', and 'Day' this field will define the frequency with which penalty points are deducted if an assignment is submitted after a due date.
- Penalty Point Per Unit - This is the amount of points which will be deducted from the student's score for the assignment round every time the Penalty Unit time has elapsed between the due date and the submission date. [Range: > 50]
- Maximum Penalty - Points will continue to be deducted from the assignment round score for each Penalty Unit time that has elapsed, but the number of points deducted will not exceed the Maximum Penalty [Range: 1..50]
Change specifics
The penalty_helper.rb and late_policies_controller.rb files are central to the creation and maintenance of late policies. Late policies are created by instructors and associated with assignments. The late_policies_controller.rb file is primarily used in the communication with view where late policies are displayed, created, or modified. The penalty_helper.rb file includes many useful methods related to late policies and the penalties which result from their use. For instance, if an assignment is designated as having penalties calculated then when it is graded the PenaltyHelper module will allow the penalties for each assignment to be calculated and applied to the final grade for the assignment.
This effort was initiated with the intent of refactoring some longer methods with redundant subprocedures so that the overall structure of the code is cleaner, concise, and DRYer. This was achieved by performing analysis on the files to identify methods which are not used within the Expertiza code base, methods which are too long, and redundant code which can be extracted from other methods and replaced with a call to a single method.
Late Policies Controller
Specific changes identified and made here, including the motivation behind making those changes.
Penalty Helper
This is a helper class which contains methods which calculate the different penalties, and are then brought together in one Hash object. The aim was to keep the methods below 25 lines of code if possible. Out of the numerous methods in the file, there were 3 substantial methods, namely calculate_penalty, calculate_submission_penalty, and compute_penalty_on_reviews crossed the 25 line limit.
calculate_penalty is the main method of the PenaltyHelper module. It is called from the GradesController module. The other methods are called from within the calculate_penalty module. The method mainly contains a few assignment statements, and some method calls. There was no scope for it to be shortened further without risking non-operation of some functionalities.
A particular segment of code converted a time difference into respective unit. This code was inserted into a method with the parameter 'time_difference'. The conversion segment was called in calculate_submission_penalty, as well as compute_penalty_on_reviews. Inserting the conversion logic into a separate method allowed the simplification of both methods.
# The new method defined to convert time difference into respective unit def calculate_penalty_minutes(time_difference) if @penalty_unit == 'Minute' penalty_minutes = time_difference / 60 elsif @penalty_unit == 'Hour' penalty_minutes = time_difference / 3600 elsif @penalty_unit == 'Day' penalty_minutes = time_difference / 86_400 end end
The method was called in two places.
# Line 44 time_difference = last_submission_time - submission_due_date penalty_minutes = calculate_penalty_minutes(time_difference)
# Line 109 time_difference = review_map_created_at_list.at(i) - review_due_date penalty_minutes = calculate_penalty_minutes(time_difference)
Apart from the method which was defined, some other methods which were not being called were removed, extraneous statements which were written and then commented were also removed. Also, minor modifications in places where code did not follow Ruby grammar were made, for eg: a space was added between 'if' and the condition statement.
Functional and integration testing
Motivation behind testing including the technologies used for testing.
penalty_helper_spec.rb
Specific changes identified and made here, including the motivation behind making those changes.
late_policies_controller_spec.rb
Specific changes identified TO BE made here, including the motivation behind making those changes.
Peer review validation
Validating application functionality
Instructions on how our reviewers should validate the impacted functionality to ensure that it still works.
Validating code via RSpec tests
Setup Expertiza VCL environment
First, setup your Expertiza environment. Navigate to https://vcl.ncsu.edu and click the "Make a Reservation" button. Sign in with your Unity ID. Click the "Reservations" button and then click "New Reservation". In the environment drop down box select the ' [CSC517, F15] Ruby on Rails / Expertiza' environment, set your session duration, and click the "Create Reservation" button.
One your reservation is ready you should see a Connect! button. Click it and copy the IP address. If you don't have PuTTY then download it. Open PuTTY and paste the IP address in the hostname field and click "Open." Confirm connection at the security dialog. Log into the server using your Unity ID and password. Once logged in, execute the following commands to setup the Expertiza environment.
git clone https://github.com/michaelamoran/expertiza.git cd expertiza cp ./config/database.yml.example ./config/database.yml cp ./config/secrets.yml.example ./config/secrets.yml vi ./Gemfile # Search for 'pg' and delete the line and save bundle install rake db:migrate curl https://raw.githubusercontent.com/creationix/nvm/v0.13.1/install.sh | bash source ~/.bash_profile nvm install v7.7.3 npm install -g bower bower install # The following commands will prepare the test database so the tests don't fail. rake db:create RAILS_ENV=test rake db:migrate RAILS_ENV=test
Executing tests
Now, to run the tests, make sure you are in the project's root directory (i.e. ~/expertiza/). Run the following command:
rspec spec/helpers/penalty_helper_spec.rb
Once it is complete, the output should contain the number of test cases and how man of them passed or failed.