CSC/ECE 517 Fall 2014/OSS E1452 slj

From Expertiza_Wiki
Jump to navigation Jump to search

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")<ref>http://en.wikipedia.org/wiki/Don%27t_Repeat_Yourself</ref> 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

  • The import/export functionality for different team participants could be combined and simplified. This responsibility could probably be broken out into separate classes so that this responsiblity is encapsulated better.
  • Rather than extending the Team class to create separate classes to capture related functionality when teams are associated with assignments or courses, it might be better to just associate teams with courses or assignments directly and then delegate related functionality (like importing/exporting data) to separate helper classes.

Conclusion

  • The renaming and refactoring was performed as suggested in the given requirements. These changes helped enforce consistency and common conventions.
  • Some common functionality was combined to adhere to the DRY principle.
  • Methods were grouped and organized within their model classes to make them easier to read and mentally consume, and to suggest groups of responsibilities for potential future work.

References

<references/>

See Also

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2014/OSS_E1452_slj&oldid=91356"