CSC/ECE 517 Fall 2014/OSS E1452 slj: Difference between revisions
No edit summary |
|||
Line 42: | Line 42: | ||
=UML Class Diagram= | =UML Class Diagram= | ||
[[File:Writing 2 UML.png]] | [[File:Writing 2 UML.png]] | ||
=Team Model: | =Team Model: team.rb= | ||
{| class="wikitable" | {| class="wikitable" | ||
|- | |- | ||
Line 49: | Line 49: | ||
! style="width:43%;"|Reason For Change | ! style="width:43%;"|Reason For Change | ||
|- style="vertical-align:top;" | |- style="vertical-align:top;" | ||
| | | get_participants | ||
| | | Removed more complex version of two duplicate functions. | ||
| | | Adherence to the DRY principle. | ||
|- | |- | ||
| .. | | add_member | ||
| ... | | Renamed to 'add_participant' | ||
| .... | | Enforce consistency in how we refer to users in a team. | ||
|- | |||
| copy_members | |||
| Renamed to 'copy_participants' | |||
| Enforce consistency in how we refer to users in a team. | |||
|- | |||
| delete_all_by_parent | |||
| Changed logic from 'for team in teams ... end' to 'teams.each { |team| ... }' | |||
| Consistency in accessing collection members. | |||
|- | |||
| delete_all_by_parent | |||
| Changed logic from 'for i in 1..no_of_teams' to '(1..no_of_teams).each { |i|' | |||
| Consistency in accessing collection members. | |||
|- | |- | ||
|} | |} | ||
=AssignmentTeam Model: | =AssignmentTeam Model: assignment_team.rb= | ||
{| class="wikitable" | {| class="wikitable" | ||
|- | |- |
Revision as of 16:49, 31 October 2014
E1452: Refactoring AssignmentTeam and CourseTeam models
The requirements provided for the Open Source System project of Expertiza were as follows:
Classes:
- assignment_team.rb
- course_team.rb
- team.rb
What they do: The AssignmentTeam model checks whether a team has participants for an assignment, whether a topic has been picked by the team and reviews has be given for a particular team and adds, deletes and retrieves participants in a team. The CourseTeam model gives the paticipants in a course and allows to add and delete participants in a course. This is for the use by the instructor.
What is needed: Check if we need two methods participant and get_participant as they may be similar. If so, refactor to make one method. Rename create_team_and_node method as create method (follow Ruby style) Rename includes? method to has_participant? Remove duplicate code from these 2 sublcasses of Team. Assignment_Team class too long (235 lines)
Introduction
The AssignmentTeam and CourseTeam models, as sibling subclasses of the Team model, tended to share certain functionality. This assignment called for refactoring those classes to make them better conform to ruby convention and to support the DRY ("Don't Repeat Yourself") principle of avoiding duplicate code.
A quick skim of the code turned up a few places where the AssignmentTeam and CourseTeam models had similar functionality, as well as multiple methods and variables that needed better names. Some preliminary suggestions were run past the point of contact for this section of the codebase. They are summarized as follows:
- The 'participants' and 'get_participants' methods in AssignmentTeam appear to have the same functionality, so these should be refactored into a single method.
- The 'create_team_and_node' method will be renamed to 'create' to better follow accepted Ruby conventions.
- A few other methods that could also be named better. For example, 'remove_team_by_id(id)' could be 'delete(id)'.
- The 'includes?' method will be renamed to 'has_participant?'
- There are some common responsibilities in both AssignmentTeam and CourseTeam, so that code will be pushed up into the superclass Team.
Project Design and Approach
This project took a two-stage approach. The first stage involved renaming methods and variables, but largely avoiding invasive code changes that could have an impact on the functionality. The second stage separately made changes like merging or removing duplicate functions.
Several main goals were kept in mind throughout this process:
- DRY
- Ruby naming conventions
- Project style conventions
- Good design cosiderations, such as high cohesion and low coupling.
UML Class Diagram
Team Model: team.rb
Method Name | Changes Made | Reason For Change |
---|---|---|
get_participants | Removed more complex version of two duplicate functions. | Adherence to the DRY principle. |
add_member | Renamed to 'add_participant' | Enforce consistency in how we refer to users in a team. |
copy_members | Renamed to 'copy_participants' | Enforce consistency in how we refer to users in a team. |
delete_all_by_parent | team| ... }' | Consistency in accessing collection members. |
delete_all_by_parent | i|' | Consistency in accessing collection members. |
AssignmentTeam Model: assignment_team.rb
Method Name | Changes Made | Reason For Change |
---|---|---|
get_first_member | Renamed to get_first_participant | Enforce consistency in how we refer to users in a team. |
includes? | Renamed to has_participant | Enforce consistency in how we refer to users in a team. |
get_participants | Renamed to participants | Enforce consistency in how we name Ruby functions. |
create_team_and_node | Renamed to create | Enforce consistency in how we name Ruby functions. |
remove_team_by_id | Renamed to delete | Enforce consistency in how we name Ruby functions. |
reviewed_contributor? | Changed logic from '(...) == false' to '!(...)' | Legibility. |
get_hyperlinks | Renamed team_member local variable to 'participant' | Enforce consistency in how we refer to users in a team. |
CourseTeam Model: course_team.rb
Method Name | Changes Made | Reason For Change |
---|---|---|
create_team_and_node | Renamed to create | Enforce consistency in how we name Ruby functions. |
copy_members | Renamed to copy_participants | Enforce consistency in how we refer to users in a team. |