CSC/ECE 517 Fall 2013/ch2 0e808 nsv
E808: Refactor and test Participant, AssignmentParticipant, & CourseParticipant
The requirements provided for the Open Source System project were as follows :
Classes:
participant.rb (197 lines)
assignment_participant.rb (476 lines)
course_participant.rb (90 lines)
What they do:
These classes keep track of a user who is participating in an assignment or a course. participant.rb is the superclass, and the other two are the subclasses.
What is needed:
The methods are complex; there are a lot of them (assignment_particpant has 44) and there is a lot of duplicated code. assignment_participant contains code for detecting cycles of high grades (which can indicate collusion in reviews). It would be better moved to another class.
Introduction
A cursory analysis of the code of the three model classes showed the existence of code duplication. In a lot of the cases the Don't Repeat Yourself (DRY) principle was violated.
In keeping with the Agile methodology, which involves having regular discussions with all stakeholders, the design check for the project was performed with a meeting with the Instructor. This helped set a direction to the way ahead for the project. The process followed in detail is explained in the following sections.
Project Design and Approach
Since the Agile Method of software development was adopted for this project, the first basic principle was accepting the fact that not all the requirements would be known at the onset , but instead would change with time. Also the primary purpose of the project was to get acquainted with the techniques needed to understand a huge application already in place and coded by others.
With this in mind, there was the freedom to propose an array of changes and then through trial and errors perceive their feasibility and impact on the application.
During the first design check with the Instructor, all the methods in the three models were parsed through and the existing issues were identified. What was also discussed was functionality that could be improved upon or new features that could be added provided these improved the overall quality of the code or made better design sense.
The result of this meeting was a spreadsheet with all model methods, their functions and proposed changes. Following through with getting timely feedback from all stakeholders, this proposal was discussed with the Instructor and modified as per comments received.
With the first set of requirements ready, the code could now be worked upon.
UML Class Diagram
A subset of the UML class diagram including only the related classes for this project can be drawn as shown below.
Participant is the superclass for subclasses AssignmentParticipant and CourseParticipant. In the database to actually distinguish between these a type attribute is used which can take values as 'AssignmentParticipant' or 'CourseParticipant' while a participant that does not belong to either an assignment nor a course cannot exist in the system as he just becomes a user and not a participant.
A participant has a user i.e. only a user can become a participant.
An AssignmentParticipant has an assignment and a CourseParticipant has a course. This relationship is defined by the parent_id attribute of a participant.
Participant model: participant.rb
The Participant model class being the base class should only contain code or methods that are relevant or needed by both the subclasses. But instead , it was cluttered with methods that would be better placed in one of the child classes as they are relevant to only that subclass. Such methods were moved to the respective sub-classes. Also certain functionalities that were common between both subclasses were repeated in both leading to code repetition. Such methods were moved to Participant class and will be dealt with in the later sections of this article. Several methods in Participant model required renaming and refactoring to ensure that the change is reflected in the entire project. The names were changed to follow method naming convention. Deprecated code was removed and re-coded to ensure that the code worked in future versions of Rails as well.
Sr. No. | Method Name | Changes Made | Reason For Change |
---|---|---|---|
1 | able_to_submit | It has been commented out. | Method not called or used anywhere. Also the method just returned the value of the attribute submit_allowed which is already taken care of by attr_accessors. |
2 | force_delete | The deprecated code in this method has been changed. | To ensure the code works in future versions of Rails. |
3 | get_average_question_score | Renamed to average_question_score and refactored. | To conform to method naming convention. |
4 | get_average_score | Renamed to average_score and refactored also moved to the model assignment_participant.rb. Removed empty parentheses from declaration | The new name is more in keeping with method naming conventions. Removal of empty parentheses helps with poetry mode of code. The method was moved because calculation of average score is only relevant to AssignmentParticipant and not to CourseParticipant. |
5 | get_current_stage | Moved to the model - assignment_participants.rb | The method was moved because getting current stage of an assignment is only relevant to AssignmentParticipant and not to CourseParticipant. |
6 | get_stage_deadline | Moved to model - assignment_participant.rb | Just like get_current_stage, this method too is only relevant to AssignmentParticipant and not to CourseParticipant. |
7 | get_topic_string | Renamed to topic_string and refactored. | To follow method naming convention |
8 | review_response_maps | Moved to model - assignment_participant.rb | Reviews are made only on assignments and hence ReviewResponseMaps would only exist for between the participants of an assignment. Thus this method would be better placed in AssignmentParticipant. |
After changes were made, the existing tests ran successfully.
AssignmentParticipant model: assignment_participant.rb
Several methods in AssignmentParticipant model class required renaming and refactoring to ensure that the change is reflected in the entire project. The names were changed to follow method naming conventions in Ruby. Deprecated code was removed and re-coded to ensure that the code worked in future version of Rails as well. Duplicate codes were commented out. Methods that are more general in the sub classes were moved to the parent class. Some methods that calculate the collusion in reviews between students were moved to a newly created model class called CollusionCycle in file collusion_cycle.rb .
Sr. No. | Method Name | Changes Made | Reason For Change |
---|---|---|---|
1 | get_course_string | Renamed to course_string and refactored | To follow method naming convention |
2 | get_cycle_deviation_score | Moved to collusion_cycle.rb model class | Related methods that calculate collusion in reviews have been moved to the newly created collusion_cycle.rb model class |
3 | get_cycle_similarity_score | Moved to collusion_cycle.rb model class | Related methods that calculate collusion in reviews have been moved to the newly created collusion_cycle.rb model class |
4 | get_feedback | Renamed to feedback and refactored | To follow method naming convention |
5 | get_files | Renamed to files_in_directory and refactored | To follow method naming convention |
6 | get_four_node_cycles | Moved to collusion_cycle.rb model class | Related methods that calculate collusion in reviews have been moved to the newly created collusion_cycle.rb model class |
7 | fullname | Moved to parent model class – participant.rb | General methods of the subclass are moved to Parent class |
8 | get_metareviews | Renamed to metareviews and refactored | To follow method naming convention |
9 | name | Commented the method | The method is already in parent model class – participant.rb and can be used in this subclass through inheritance |
10 | name | Commented the method | The method is already in parent model class – participant.rb and can be used in this subclass through inheritance. Also it is a duplicate method in patricipant.rb |
11 | get_review_score | Renamed to review_score and refactored. Deprecated code has been removed and replaced with correct code. | To follow method naming convention and ensure the code works in future versions. |
12 | get_reviewees | Renamed to reviewees and refactored. Deprecated code has been removed and replaced with correct code. | To follow method naming convention and ensure the code works in future versions. |
13 | get_reviewers | Renamed to reviewers and refactored . Deprecated code has been commented and replaced with correct code. | To follow method naming convention and ensure the code works in future versions. |
14 | get_reviews | Renamed to reviews and refactored | To follow method naming convention |
15 | get_reviews_by_reviewer | Renamed to reviews_by_reviewer and refactored | To follow method naming convention |
16 | get_reviews_by_reviewer | Commented the method | To remove duplicated method in this class |
17 | get_scores | Renamed to scores and refactored | To follow method naming convention |
18 | get_submitted_files | Renamed to submitted_files and refactored | To follow method naming convention |
19 | get_teammate_reviews | Renamed to teammate_reviews and refactored | To follow method naming convention |
20 | get_three_node_cycles | Moved to collusion_cycle.rb model class | Related methods that calculate collusion in reviews have been moved to the newly created collusion_cycle.rb model class |
21 | get_topic_string | Method has been Commented | It is already present in model super class – participant.rb |
22 | get_two_node_cycles | Moved to collusion_cycle.rb model class | Related methods that calculate collusion in reviews have been moved to the newly created collusion_cycle.rb model class |
23 | get_wiki_submissions | Renamed to wiki_submissions and refactored | To follow method naming convention |
After changes were made, the existing tests ran successfully.
CourseParticipant model:course_participant.rb
The CourseParticipant model has methods like get_path and self.get_export_fields which were present in both assignment_participant.rb and course_participant.rb. Since they had dynamic usages, we could not delete them from the two subclasses and place in participant.rb.
Sr. No. | Method Name | Changes Made | Reason For Change |
---|---|---|---|
1 | get_course_string | Renamed to course_string and refactored | To follow method naming convention. |
Changes to yaml files
Testing
Future scope
Changes in routes.rb
The following missing routes were added to the file routes.rb to make the program flow smoothly.
Routes for submit_hyperlink and remove_hyperlink were added to the submitted_content controller as :
resources :submitted_content do collection do get :view get :edit post :submit_hyperlink get :remove_hyperlink end end
Routes for the get and post methods of the export_file controller were added :
resources :export_file do collection do get :start get :export post :export end end
Routes for cancel, accept and decline were added to the invitation controller :
resources :invitation do collection do get :cancel get :accept get :decline end end
Conclusion
.
References
<references/>
See Also
- OSS - E808 project VCL link http://152.7.99.84:3000/ (may be disabled after a month)
- OSS - E808 project on GitHub https://github.com/npari/expertiza_oss808
- Expertiza project documentation can be found at http://wikis.lib.ncsu.edu/index.php/Expertiza
- The working Expertiza site is accessible at link http://expertiza.ncsu.edu/
- Expertiza on GitHub https://github.com/expertiza/expertiza