CSC 517 S2016 E1611 Refactor team.rb, course team.rb, and assignment team.rb: Difference between revisions
No edit summary |
No edit summary |
||
Line 36: | Line 36: | ||
=== Solutions Provided === | === Solutions Provided === | ||
* '''Problem 1:''' | * '''Problem 1:''' | ||
This method was refactored to the parent class team.rb from the assignment_team.rb and the course_team.rb. The new code is : | Import :This method was refactored to the parent class team.rb from the assignment_team.rb and the course_team.rb. The new code is : | ||
< | <pre> | ||
def self.import(row, id, options,teamtype) | def self.import(row, id, options,teamtype) | ||
raise ArgumentError, "Not enough fields on this line" if (row.length < 2 && options[:has_column_names] == "true") || (row.length < 1 && options[:has_column_names] != "true") | raise ArgumentError, "Not enough fields on this line" if (row.length < 2 && options[:has_column_names] == "true") || (row.length < 1 && options[:has_column_names] != "true") | ||
Line 68: | Line 68: | ||
team.import_team_members(index, row) if !(team_exists && options[:handle_dups] == "ignore") | team.import_team_members(index, row) if !(team_exists && options[:handle_dups] == "ignore") | ||
end | end | ||
</ | </pre> | ||
Export :This method was refactored to the parent class team.rb from the assignment_team.rb and the course_team.rb. The new code is : | |||
<pre> | |||
def self.export(csv, parent_id, options, teamtype) | |||
if teamtype.is_a?(CourseTeam) | |||
teams = CourseTeam.where(["parent_id =?", parent_id]) | |||
elsif teamtype.is_a?(AssignmentTeam) | |||
teams = AssignmentTeam.where(["parent_id =?", parent_id]) | |||
end | |||
teams.each do |team| | |||
output = Array.new | |||
output.push(team.name) | |||
if options["team_name"] == "false" | |||
team_members = TeamsUser.where(['team_id = ?', team.id]) | |||
team_members.each do |user| | |||
output.push(user.name) | |||
end | |||
end | |||
output.push(teams.name) | |||
csv << output | |||
end | |||
end | |||
</pre> | |||
Helper Code: Some of the helper methods also had to be refactored to the parent class. | |||
<pre> | <pre> | ||
def self.create_team_and_node(id,teamtype) | |||
if teamtype.is_a?(CourseTeam) | |||
curr_course = Course.find(id) | |||
team_name = Team.generate_team_name(curr_course.name) | |||
team = CourseTeam.create(name: team_name, parent_id: id) | |||
TeamNode.create(parent_id: id, node_object_id: team.id) | |||
elsif teamtype.is_a?(AssignmentTeam) | |||
curr_assignment = Assignment.find(id) | |||
team_name = Team.generate_team_name(curr_assignment.name) | |||
team = AssignmentTeam.create(name: team_name, parent_id: id) | |||
TeamNode.create(parent_id: id, node_object_id: team.id) | |||
end | |||
team | |||
end | |||
---- | |||
def self.handle_duplicate(team, name, id, handle_dups, teamtype) | |||
if team.nil? #no duplicate | |||
return name | |||
end | |||
if handle_dups == "ignore" #ignore: do not create the new team | |||
p '>>>setting name to nil ...' | |||
return nil | |||
end | |||
if handle_dups == "rename" #rename: rename new team | |||
if teamtype.is_a?(CourseTeam) | |||
return self.generate_team_name(Course.find(id).name) | |||
elsif teamtype.is_a?(AssignmentTeam) | |||
return self.generate_team_name(Assignment.find(id).name) | |||
end | |||
end | |||
if handle_dups == "replace" #replace: delete old team | |||
team.delete | |||
return name | |||
else # handle_dups = "insert" | |||
return nil | |||
end | |||
end | |||
</pre> | </pre> | ||
=== Testing from UI === | === Testing from UI === |
Revision as of 01:40, 24 March 2016
E1611 Refactor team.rb, course_team.rb, and assignment_team.rb
This page provides the description of the Expertiza based OSS project on refactoring.
Introduction to Expertiza
Expertiza is an open source project developed on Ruby on Rails framework. It is primarily developed as a platform for instructors to create, assign and grade assignments and quizzes and for the students to view, submit assignments and take quizzes. Other activities involved in a course can also be done via Expertiza, like creating teams, reviewing assignments among others.
Problem Statement
The three classes in question- team, assignment team and course team, deal with teams of students that do assignments."team.rb" is the superclass; course_team is a team created for a course (if students are expected to stay in the same team all semester), and assignment_team is a team created for an assignment. Either the instructor can set up course or assignment teams (using the “Create teams” icon on the course or assignment Actions menu), or the students can, by issuing and accepting invitations.
The code for 'importing', 'exporting', and 'copying' teams for assignment and course violates the DRY principle. There is a paucity of comments and there are deprecated methods and other methods that may not be used anymore, and could be deleted.
Tasks Identified
The following tasks were accomplished in this project:
- Create a common method for importing and exporting teams in the team.rb file instead of having 2 separate functions in the assignment_team.rb and course_team.rb files using the prototype pattern.
- Move the create_team_and_method to superclass (team.rb).
- Delete deprecated methods.
- Rewrite the method for checking if a team has submission.
- Remove unnecessary white spaces and add comments before methods to describe what the method does.
System Design
This project deals with the "team" module of Expertiza which handles the creation of teams and other aspects related to teams like submission, importing and exporting. Expertiza has 3 models, team.rb, course_team.rb and assignment_team.rb for teams in general, teams for a course and teams for each assignment.
The design specific to these models is as follows::
Here the course_team and assignment_team are subclasses of team. The parent_id field in the database of teams refer to the id of course or assignment based on the type for which it was created. Hence in course_team instances the parent_id contains the course ID and for assignment_team the parent_id contains the assignment ID. The main table stores the team members is TeamUsers, where each line is uniquely identified by the user_id and the team_id. This table is linked to Team, Participant & Users.
The understanding of this system design is core to understanding the functionality of team.rb, course_team.rb and assignment_team.rb models.
Solutions Provided
- Problem 1:
Import :This method was refactored to the parent class team.rb from the assignment_team.rb and the course_team.rb. The new code is :
def self.import(row, id, options,teamtype) raise ArgumentError, "Not enough fields on this line" if (row.length < 2 && options[:has_column_names] == "true") || (row.length < 1 && options[:has_column_names] != "true") if options[:has_column_names] == "true" name = row[0].to_s.strip team = where(["name =? && parent_id =?", name, id]).first team_exists = !team.nil? name = handle_duplicate(team, name, id, options[:handle_dups],teamtype) index = 1 else if teamtype.is_a?(CourseTeam) name = self.generate_team_name(Course.find(id).name) elsif teamtype.is_a?(AssignmentTeam) name = self.generate_team_name(Assignment.find(id).name) end index = 0 end if name team = Team.create_team_and_node(id,teamtype) team.name = name team.save end # insert team members into team unless team was pre-existing & we ignore duplicate teams team.import_team_members(index, row) if !(team_exists && options[:handle_dups] == "ignore") end
Export :This method was refactored to the parent class team.rb from the assignment_team.rb and the course_team.rb. The new code is :
def self.export(csv, parent_id, options, teamtype) if teamtype.is_a?(CourseTeam) teams = CourseTeam.where(["parent_id =?", parent_id]) elsif teamtype.is_a?(AssignmentTeam) teams = AssignmentTeam.where(["parent_id =?", parent_id]) end teams.each do |team| output = Array.new output.push(team.name) if options["team_name"] == "false" team_members = TeamsUser.where(['team_id = ?', team.id]) team_members.each do |user| output.push(user.name) end end output.push(teams.name) csv << output end end
Helper Code: Some of the helper methods also had to be refactored to the parent class.
def self.create_team_and_node(id,teamtype) if teamtype.is_a?(CourseTeam) curr_course = Course.find(id) team_name = Team.generate_team_name(curr_course.name) team = CourseTeam.create(name: team_name, parent_id: id) TeamNode.create(parent_id: id, node_object_id: team.id) elsif teamtype.is_a?(AssignmentTeam) curr_assignment = Assignment.find(id) team_name = Team.generate_team_name(curr_assignment.name) team = AssignmentTeam.create(name: team_name, parent_id: id) TeamNode.create(parent_id: id, node_object_id: team.id) end team end ---- def self.handle_duplicate(team, name, id, handle_dups, teamtype) if team.nil? #no duplicate return name end if handle_dups == "ignore" #ignore: do not create the new team p '>>>setting name to nil ...' return nil end if handle_dups == "rename" #rename: rename new team if teamtype.is_a?(CourseTeam) return self.generate_team_name(Course.find(id).name) elsif teamtype.is_a?(AssignmentTeam) return self.generate_team_name(Assignment.find(id).name) end end if handle_dups == "replace" #replace: delete old team team.delete return name else # handle_dups = "insert" return nil end end