CSC/ECE 517 Fall 2015/ossE1558BGJ
Introduction
This page provides a description of the modifications and improvements made to the Expertiza project’s source code as part of an Expertiza based Open Source Software project. Expertiza is a web based application which allows students to submit and peer-review classmates’ work, including written articles and development projects. The specific changes made during the course of this project were to the ReviewResponseMap.rb file, with additional changes made to other related files in support of refactoring ReviewResponseMap.rb.
Problem Statement
ReviewResponseMap is the model class used to manage the relationship between contributors, reviewers, and assignments. The intent of the changes were to refactor the code for better readability and adherence to Ruby coding standards and best practices. Primarily these changes involved the refactoring of overly complex methods, renaming methods for better readability, and the addition of missing unit tests.
Project Requirements:
- Code Climate shows import method is complex, because of lots of checks. This method can be fixed by adding private methods for raising import error.
- Get_assessments_round_for method can be renamed to get_team_responses_for_round. Team_id private variable is not needed.
- metareview_response_maps rename, refactoring can be done. No need to do second iteration.
- write missing unit tests for existing methods.
Changes
Refactoring of import method
Code Climate reports showed the import method to be overly complex, with a number of checks being included in the import method itself. The resolution for this problem was to refactor the import method, creating private methods to perform the null checks and throw import errors.
Prior to refactoring, all null checks were performed in line:
After refactoring the import method adheres to the single responsibility principle, performing only key import tasks while making calls to private methods for any necessary validation:
Private methods added:
Refactoring of get_assessments_round_for method
The get_assessments_round_for method’s name failed to provide a clear idea of the method’s purpose and functionality. Additionally, there was a private variable, team_id, introduced in the method that was unnecessary and could be replaced by using the id property of the team parameter directly.
Following the refactoring, the name of the method provides a clearer idea of its purpose:
The change in method name also required changes to the following files that referenced it:
/app/models/assignment.rb:436:
./app/models/assignment_participant.rb:268:
Refactor metareview_response_maps
The metareview_response_maps method was unnecessarily complex, introducing unneeded private variables and an additional iteration inside the main loop.
After refactoring:
Addition of code comments and unit tests
In addition to the refactoring performed to DRY up the ReviewResponseMap.rb methods we added missing tests for the existing methods and restructured the pre-existing tests to use fixtures. The following fixture files were added during the process:
- assignment_questionnaires.yml
- assignments.yml
- participants.yml
- questionnaired.yml
- response_maps.yml
- responses.yml
- teams.yml
- users.yml
Unit Tests
To run the unit tests, follow these steps:
- Download the master branch of the repo.
- Setup the Databases for the test environment (we have used Zhewei's scrubbed expertiza DB )
- Run the " rake db:create RAILS_ENV=test " command
- Run the " rake db:reset RAILS_ENV=test " command
- Scrub the DB using " mysql -u root expertiza_development < expertiza-scrubbed.sql"
- Run the " rake db:migrate "
- run " rake test test/unit/review_response_map_test.rb " in the "expertiza/" directory.
- Check if tests passed or failed.
We also added method comments to all of the methods in the ReviewResponseMap.rb file and corrected instances where CodeClimate Identified opportunities for code improvement. One issue flagged by CodeClimate that we did not change was to use the find_by method instead of where().first. However it is not always appropriate to use the find_by method, as documented in this github post:
https://github.com/bbatsov/rubocop/issues/1938
In instances where ordering by id is the desired behavior, where().first will do by default, whereas in Rails 4+ find_by does not honor that default behavior. For this reason we left the instances of where().first unchanged.