CSC/ECE 517 Fall 2014/OSS E1452 slj: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
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.rb=
=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.rb=
=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:

  1. DRY
  2. Ruby naming conventions
  3. Project style conventions
  4. 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.

Considerations for Future Work

Conclusion

References

See Also