CSC/ECE 517 Spring 2013/OSS E608: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
 
(16 intermediate revisions by 3 users not shown)
Line 1: Line 1:
[https://docs.google.com/a/ncsu.edu/document/d/1SVNx7Eh6dUdrz2a9rGSgBxLkxAmuBUm8eu08nHEdjIg/edit# Write-up of This Topic.]
[https://docs.google.com/a/ncsu.edu/document/d/1SVNx7Eh6dUdrz2a9rGSgBxLkxAmuBUm8eu08nHEdjIg/edit# Write-up of This Topic.]
__TOC__
__TOC__
= Expertiza Project =
Expertiza is a web-based class management system.  This page will discuss one of its related modification projects conducted in the CSC/ECE 517 Spring 2003 class at North Carolina State University.  Learn more about Expertiza by visiting its main page:  http://wikis.lib.ncsu.edu/index.php/Expertiza


= E608. Refactoring and Testing - Course Related Classes =
= E608. Refactoring and Testing - Course Related Classes =
Line 162: Line 166:


==== Miscellaneous Changes ====
==== Miscellaneous Changes ====
Check in part 1 is wrong because number of element can be less than 2 if the array does not have a team name column.
The condition in Part 1 is incorrect because the number of elements could be less than 2 if the array does not have a team name column.


  if (row.length < 2 and options[:has_column_names] == "true") or (row.length < 1 and options[:has_column_names] != "true")
  if (row.length < 2 and options[:has_column_names] == "true") or (row.length < 1 and options[:has_column_names] != "true")
Line 175: Line 179:


==== Other Issues ====
==== Other Issues ====
The input variable session was unused in the method therefore it should be removed, however due to the way that the method is being called in the import file controller this change was not made in this project.
The input variable session was unused in the method; therefore, it needed to be removed.  However, this change was not made in this project due to the way that the method is being called in the import file controller.


=== ''export'' Method ===
=== ''export'' Method ===
==== Functionality Bug ====
==== Functionality Bug ====
Originally the export method in the CourseTeam class export the assignment teams that are associated with the course that a course team belongs.  This is not the behavior that one would expect from the export method in the CourseTeam class.  
Originally, the export method in the CourseTeam class exported the assignment teams that were associated with the course to which a course team belonged.  This is not the behavior that one would expect from the export method in the CourseTeam class.  


Upon examining the AssignmentTeam class and confirming with the professor we discover that the export method for CourseTeam should export all of the course teams associated with a course.  
Upon examining the AssignmentTeam class, the export method for CourseTeam was determined to export all of the course teams associated with a course.  


  def self.export(csv, parent_id, options)
  def self.export(csv, parent_id, options)
Line 202: Line 206:
   end
   end


Note that this is a director port from the source of export function in the AssignmentTeam class
Note that this is a direct port of code from the source of the export function in the AssignmentTeam class.


The original export function was rename to export_all_assignment_team_related_to_course to reflect its actually functionality  
The original export function was renamed to export_all_assignment_team_related_to_course to reflect its actual functionality.  The function was marked deprecated and if the functionality is later found to be needed, the function should be re-written and moved to the Course class.
the function was marked deprecated and if the functionality was truly needed we suggest that function should be re-written and move to the Course class


==== Analysis and Refactor ====
==== Analysis and Refactor ====
The part of the export method within the if block is use to export all of the participants of a team this part can be separated an turn into its own method to reduce the complexity of the export method
The part of the export method within the if block is used to export all of the participants of a team.  This part can be separated into its own method to reduce the complexity of the export method.


   def export_participants
   def export_participants
Line 220: Line 223:
   end
   end


Furthermore, an instance method was added to the CourseTeam class to export the team and the class method export can call the instance method to reduce the complexity and improve the readability.
Furthermore, an instance method was added to the CourseTeam class to export the team.  The class method export can call the instance method to reduce the complexity and improve the readability.


   def export(team_name_only)
   def export(team_name_only)
Line 233: Line 236:
   end
   end


The export function after refactoring  
The export function as it looked after refactoring:


   def self.export(csv, parent_id, options)
   def self.export(csv, parent_id, options)
Line 247: Line 250:


==== Other Issues ====
==== Other Issues ====
the method name export does not accurately describe the functionality of this method. We suggest that the name should be change to export_all. This change was not made due to the export method in the AssignmentTeam class and the way that the method is being called by the export file controller.
The method name export does not accurately describe the functionality of this method. It is recommended that this method name be changed to export_all. This change was not made due to the export method in the AssignmentTeam class and the way that the method is being called by the export file controller.


The meaning of the option team_name in the method is ambiguous. Upon further examination we discover that the option team_name control if the method should only export team names without exporting the name of the participants in the team. This option should be renamed to team_name_only for better clarity.
The meaning of the option team_name in the method is ambiguous. Upon further examination, it was discovered that the option team_name controls if the method should only export team names without exporting the name of the participants in the team. This option should be renamed to team_name_only for better clarity.


=== ''get_export_fields'' Method ===
=== ''get_export_fields'' Method ===
Inferring from the code we think that get_export_field method supposed to returns an array that contains the options that used for controlling the export method.
Inferring from the code, the get_export_field method seems to return an array that contains the options used for controlling the export method.


As we discussed earlier the team_name option for expert method controls if the export should contain the name of the team participants.
As discussed earlier, the team_name option for expert method controls if the export should contain the name of the team participants.


Originally the get_export_fields method will include the string “team member” if the option team_name is true. This is the opposite of what we expected if our interpretation of the option team_name is correct.  
Originally, the get_export_fields method will include the string “team member” if the option team_name is true.  


Further more the get_export_fields method is trying to get the value of string(“team_name”) in hash variable options instead of symbol(:team_name).
Furthermore, the get_export_fields method is trying to get the value of string(“team_name”) in hash variable options instead of symbol(:team_name).


After consulting with the professor we confirm that our suspicion is right and made the change accordingly
Here is the method after making the noted changes:


   def self.get_export_fields(options)
   def self.get_export_fields(options)
Line 272: Line 275:


=== ''copy'' Method ===  
=== ''copy'' Method ===  
From reading the code we inferred that this method is used to create a new assignment team and copy all of the participants of the course team to that new assignment team.
This method is inferred to be used to create a new assignment team and copy all of the participants of the course team to that new assignment team.


If this is truly the intended functionality of this method than this method should belongs to AssignmentTeam class instead of CourseTeam class.
If this is later confirmed to be the intended functionality of this method, then this method should belong to AssignmentTeam class instead of CourseTeam class.


This method calls the create_node_object method in Team class which does not work (explained in the later section) and has ambiguous return type, functionality and name.
This method calls the create_node_object method in Team class, which does not work (explained in a later section) and has an ambiguous return type, functionality, and name.


Logically the copy method should create a new course team and copy the participants of a team into the new course team.
Logically, the copy method should create a new course team and copy the participants of a team into the new course team.
Currently this is dead code and not used in the program.   
Currently, this is dead code and not used in the program.   


This function was marked deprecated and we suggest that the function should be removed from the program.
This function was marked deprecated and it is recommended to be removed from the program.


=== ''add_participant'' Method ===  
=== ''add_participant'' Method ===  
From reading the code we inferred that this method is used to add a participant to the course that the course team belongs to.
This method is used to add a participant to the course that the course team to which it belongs.


This method is called by the add_member method in the Team class to add a participant to the course when a member is added to a course team.
This method is called by the add_member method in the Team class to add a participant to the course when a member is added to a course team.
Line 290: Line 293:
This method does belong in the Course class and not the CourseTeam class.  The Course class already has the method add_participant with similar functionality.
This method does belong in the Course class and not the CourseTeam class.  The Course class already has the method add_participant with similar functionality.


This function was marked deprecated and we suggest that the function should be removed from the program and the add_member method in the Team class should call the add_participant method from Course class.
This function was marked deprecated and it is recommended that the function should be removed from the program and the add_member method in the Team class should call the add_participant method from Course class.


== Changes for /app/models/course_node.rb ==
== Changes for /app/models/course_node.rb ==
Line 318: Line 321:
* /app/models/course_node.rb
* /app/models/course_node.rb
** CourseNode.get_modified_date
** CourseNode.get_modified_date
== Miscellaneous Changes for Quality Improvement and Readability ==


== Other Observations and Suggestion for Future Work ==  
== Other Observations and Suggestion for Future Work ==  
Line 327: Line 328:
Files: team.rb, course_team.rb, assignment_team.rb
Files: team.rb, course_team.rb, assignment_team.rb


The class CourseTeam and AssignmentTeam both inherits from the class Team. However that are a lot of duplicated functionalities in order to adhere to the "DRY" principle refactor for the CourseTeam mentioned in this document should be applied to AssignmentTeam and duplicated methods should be extracted to the super-class Team.
The class CourseTeam and AssignmentTeam both inherit from the class Team. However, there are a lot of duplicated functionalities.  In order to adhere to the "DRY" principle, the refactoring mentioned earlier for the CourseTeam should be applied to AssignmentTeam and duplicated methods should be extracted to the super-class Team.


=== Naming Inconsistency in Team Related Classes ===
=== Naming Inconsistency in Team Related Classes ===
Line 334: Line 335:
Files: team.rb, course_team.rb, assignment_team.rb
Files: team.rb, course_team.rb, assignment_team.rb


Participant in a team have many different names in these three classes. For example: member, user, participants. Common vocabulary should be established in-order to eliminate confusion.  
Participants in a team have many different names in these three classes. For example: member, user, participants. Common vocabulary should be established in order to eliminate confusion.  


=== Bugs in Team Class ===
=== Bugs in Team Class ===
Line 343: Line 344:
Method: create_node_object
Method: create_node_object


The create_node_object method in Team class does not work because the class method get_parent_model in both CourseTeam and Assignment team is an instance method not a class method.
The create_node_object method in Team class does not work because the class method get_parent_model in both CourseTeam and Assignment team is an instance method and not a class method.


From the code of the object and the usage of this code we inferred that the method supposed to create a new course or assignment team and a new team node and return the newly created team as output.
Based on the code of the object and the usage of this code, the method is supposed to create a new course or assignment team along with a new team node and then return the newly created team as output. This function should be deprecated and removed from the class.  The functionality should be moved to the new method in the team controller if the functionality is found to be necessary.
This function should be deprecated and removed from the class, the functionality should be move to the new method in the team controller if the functionality is truly necessary.


=== Inelegance Code in Team Class ===
=== Inelegance Code in Team Class ===
Line 355: Line 355:
Methods: randomize_all_by_parent, check_for_existing, create_node_object, copy_members
Methods: randomize_all_by_parent, check_for_existing, create_node_object, copy_members


These methods uses Object.const_get() with a input string that dynamically locate the object that are associated with different team types (assignment or course) however the use of Object.const_get() are inconsistent throughout these methods.
These methods uses Object.const_get() with an input string that dynamically locates the object that is associated with different team types (assignment or course); however, the uses of Object.const_get() are inconsistent throughout these methods.


We suggests instead of using query methods such as get_participant_type and get_parent_model in the assignment and course team return string containing the name of the object we can use these method to return the object itself thus eliminating the need for Object.const_get().
It is recommended that instead of using query methods, such as get_participant_type and get_parent_model, in the assignment and course team to return a string containing the name of the object, these methods should be used to return the object itself.  This will eliminate the need for Object.const_get().


== List of New Unit Tests and Testcases ==
== List of New Unit Tests and Testcases ==
Line 403: Line 403:
* test/unit/course_test.rb
* test/unit/course_test.rb
** The line "fixtures :courses" was changed to "fixtures :courses,:teams,:users,:participants,:assignments,:nodes,:tree_folders".
** The line "fixtures :courses" was changed to "fixtures :courses,:teams,:users,:participants,:assignments,:nodes,:tree_folders".
= How to Run the Project =
== How to Run Expertiza ==
* Navigate to the project root directory in terminal
* run the script
ruby script/server -b 0.0.0.0 -p 3000
== How to Run the Unit Tests ==
One can either
* run the unit tests in RubyMine
or
* navigate to the project root directory in terminal and run the unit tests via this command
ruby -Itest test/unit/TEST_FILENAME.rb

Latest revision as of 20:41, 16 April 2013

Write-up of This Topic.

Expertiza Project

Expertiza is a web-based class management system. This page will discuss one of its related modification projects conducted in the CSC/ECE 517 Spring 2003 class at North Carolina State University. Learn more about Expertiza by visiting its main page: http://wikis.lib.ncsu.edu/index.php/Expertiza

E608. Refactoring and Testing - Course Related Classes

Classes Involved:

  • /app/models/course.rb (76 lines)
  • /app/models/course_participant.rb (94 lines)
  • /app/models/course_team.rb (116 lines)
  • /app/models/course_node.rb (95 lines)

What it does: Involves creation and management of courses in Expertiza

What needs to be done:

  • It is not clear what a method named 'create_node' is doing in the 'course' class. Refactor by renaming this method appropriately.
  • In the course_team class the import method seems to be doing too many things. See if this method can be broken down into smaller methods with specific functionality.
  • The 'get' method in course_node.rb appears to be doing a lot. Refactor this method by breaking it up into smaller methods.
  • Look for any unused methods or variables in these files.
  • Also apply other refactorings such as Rename variable, Rename method to give the variables and methods more meaningful names.
  • Write unit tests for all the methods in each of the listed model classes.

Changes for /app/models/course.rb

The following changes were made:

  • Since the method create_node simply creates a course node associated with the newly created or copied course, we replaced its functionality by adding the line CourseNode.create(:node_object_id => course.id) in the course_controller's create method and copy method. The function of getting the CourseNode's parent id is extracted as a class method of CourseNode model since this parent id is independent of the course. With all of its functionalities refactored to other places, this method is hereby removed.
  • Added the line "has_many :assigments, :dependent => :destroy" to allow CourseTest.test_destroy unit test to pass.

Changes for /app/models/course_participant.rb

  • CourseParticipant.get_parent_name is removed
    • This method is never called in the project. Additionally, the method CourseParticipant.get_course_string provides the same functionality as CourseParticipant.get_parent_name, so it was deleted to follow the principle of DRY.

Changes for /app/models/course_team.rb

import Method

Functionality: Import a single course team for a course from an input array. The first element in the array is optionally used to specify a name for the team that is being imported.

Usage: The method is used by the import file controller to import course teams to a course.

Decomposition

The import method can be decomposed into 5 separate parts:

Part 1 – Preparation

Functionality: Ensure that the array has enough elements and that the class in which the user wants to import the team exists.

if row.length < 2
    raise ArgumentError, "Not enough items" 
end 
course = Course.find(id)
if course == nil
    raise ImportError, "The course with id \""+id.to_s+"\" was not found. <a href='/assignment/new'>Create</a> this assignment?"
end
Part 2 – Naming the New Team

Functionality: The method will either import the team name from the array if has_column_name is set to true or generate a new team name if has_column_name is set to false.

if options[:has_column_names] == "true"
    name = row[0].to_s.strip
    index = 1
else
    name = generate_team_name()
    index = 0
end 
Part 3 – Handling Duplicate Team

Functionality: If a team with the same team name exists, this part of the method will handle the conflict based on the option handle_duplicate.

currTeam = CourseTeam.find(:first, :conditions => ["name =? and parent_id =?",name,course.id])  
if options[:handle_dups] == "ignore" && currTeam != nil
    return
end  
if currTeam != nil && options[:handle_dups] == "rename"
    name = generate_team_name()
    currTeam = nil
end
if options[:handle_dups] == "replace" && teams.first != nil        
for teamsuser in TeamsUser.find(:all, :conditions => ["team_id =?", currTeam.id])
    teamsuser.destroy
    end    
    currTeam.destroy
    currTeam = nil
end     
   
Part 4 – Create the New Team and Node

Functionality: Creates a new team and team node base on the information gathered by the above parts.

if currTeam == nil
    currTeam = CourseTeam.new
    currTeam.name = name
    currTeam.parent_id = course.id
    currTeam.save
    parent = CourseNode.find_by_node_object_id(course.id)
    TeamNode.create(:parent_id => parent.id, :node_object_id => currTeam.id)
end
Part 5 – Import the Participants to the New Team

Functionality: Verify that the users specified in the array exist, then add the users to the newly created team.

while(index < row.length) 
    user = User.find_by_name(row[index].to_s.strip)
    if user == nil
        raise ImportError, "The user \""+row[index].to_s.strip+"\" was not found. <a href='/users/new'>Create</a> this user?"                           
    elsif currTeam != nil         
        currUser = TeamsUser.find(:first, :conditions => ["team_id =? and user_id =?", currTeam.id,user.id])          
        if currUser == nil
            currTeam.add_member(user)            
        end                      
    end
    index = index+1      
end

Refactor

From the decomposition, Part 5 can be seen as needing to be separated into its own method.

 def import_participants(starting_index, row)
   index = starting_index
   while(index < row.length)
     user = User.find_by_name(row[index].to_s.strip)
     if user.nil?
       raise ImportError, "The user \""+row[index].to_s.strip+"\" was not found. <a href='/users/new'>Create</a> this user?"
     else
       if TeamsUser.find(:first, :conditions => ["team_id =? and user_id =?", id, user.id]).nil?
         add_member(user)
       end
     end
     index = index + 1
   end
 end

From the decomposition, the logic of Part 2 and Part 3 show that they could be simplified. Originally, Part 4 was executed regardless of if the team name was generated or imported. However, if the team name is generated instead of imported, there will not be another team with the same name. Therefore, we extracted Part 4 into its own method and modified the logic of Part 3.

 def self.handle_duplicate(name, course_id, handle_dups)
   team = find(:first, :conditions => ["name =? and parent_id =?", name, course_id])
   if team.nil? #no duplicate
     return name
   end
   if handle_dups == "ignore" #ignore: not create the new team
     return nil
   end
   if handle_dups == "rename" #rename: rename new team
     return generate_team_name
   end
   if handle_dups == "replace" #replace: delete old team
     team.delete
     return name
   end
 end

After the modifications to Part 3 and Part 4 are completed, the import method becomes:

   if options[:has_column_names] == "true"
     name = row[0].to_s.strip
     name = handle_duplicate(name, course_id, options[:handle_dups])
     index = 1
   else
     name = generate_team_name
     index = 0
   end

Miscellaneous Changes

The condition in Part 1 is incorrect because the number of elements could be less than 2 if the array does not have a team name column.

if (row.length < 2 and options[:has_column_names] == "true") or (row.length < 1 and options[:has_column_names] != "true")
  raise ArgumentError, "Not enough items"
end

Part 4 of the method was simplified to improve the readability.

team = CourseTeam.create(:name => name, :parent_id => course_id)
course_node = CourseNode.find_by_node_object_id(course_id)
TeamNode.create(:parent_id => course_node.id, :node_object_id => team.id)

Other Issues

The input variable session was unused in the method; therefore, it needed to be removed. However, this change was not made in this project due to the way that the method is being called in the import file controller.

export Method

Functionality Bug

Originally, the export method in the CourseTeam class exported the assignment teams that were associated with the course to which a course team belonged. This is not the behavior that one would expect from the export method in the CourseTeam class.

Upon examining the AssignmentTeam class, the export method for CourseTeam was determined to export all of the course teams associated with a course.

def self.export(csv, parent_id, options)
   currentCourse = Course.find(parent_id)
   currentCourse.teams.each { |team|
     tcsv = Array.new
     teamUsers = Array.new
     tcsv.push(team.name)
     if (options["team_name"] == "false")
       teamMembers = TeamsUser.find(:all, :conditions => ['team_id = ?', team.id])
       teamMembers.each do |user|
         teamUsers.push(user.name)
         teamUsers.push(" ")
       end
       tcsv.push(teamUsers)
     end
     tcsv.push(currentCourse.name)
     csv << tcsv
   }
 end

Note that this is a direct port of code from the source of the export function in the AssignmentTeam class.

The original export function was renamed to export_all_assignment_team_related_to_course to reflect its actual functionality. The function was marked deprecated and if the functionality is later found to be needed, the function should be re-written and moved to the Course class.

Analysis and Refactor

The part of the export method within the if block is used to export all of the participants of a team. This part can be separated into its own method to reduce the complexity of the export method.

 def export_participants
   userNames = Array.new
   participants = TeamsUser.find(:all, :conditions => ['team_id = ?', self.id])
   participants.each do |participant|
     userNames.push(participant.name)
     userNames.push(" ")
   end
   return userNames
 end

Furthermore, an instance method was added to the CourseTeam class to export the team. The class method export can call the instance method to reduce the complexity and improve the readability.

 def export(team_name_only)
   output = Array.new
   output.push(self.name)
   if team_name_only == "false"
     output.push(self.export_participants)
   end
   course = Course.find(self.parent_id)
   output.push(course.name)
   return output
 end

The export function as it looked after refactoring:

 def self.export(csv, parent_id, options)
   course = Course.find(parent_id)
   if course.nil?
     raise ImportError, "The course with id \""+course_id.to_s+"\" was not found. <a href='/assignment/new'>Create</a> this course?"
   end
   teams = CourseTeam.find_all_by_parent_id(parent_id)
   teams.each do |team|
     csv << team.export(options[:team_name])
   end
 end

Other Issues

The method name export does not accurately describe the functionality of this method. It is recommended that this method name be changed to export_all. This change was not made due to the export method in the AssignmentTeam class and the way that the method is being called by the export file controller.

The meaning of the option team_name in the method is ambiguous. Upon further examination, it was discovered that the option team_name controls if the method should only export team names without exporting the name of the participants in the team. This option should be renamed to team_name_only for better clarity.

get_export_fields Method

Inferring from the code, the get_export_field method seems to return an array that contains the options used for controlling the export method.

As discussed earlier, the team_name option for expert method controls if the export should contain the name of the team participants.

Originally, the get_export_fields method will include the string “team member” if the option team_name is true.

Furthermore, the get_export_fields method is trying to get the value of string(“team_name”) in hash variable options instead of symbol(:team_name).

Here is the method after making the noted changes:

 def self.get_export_fields(options)
   fields = Array.new
   fields.push("Team Name")
   if (options[:team_name] == "false")
     fields.push("Team members")
   end
   fields.push("Course Name")
 end

copy Method

This method is inferred to be used to create a new assignment team and copy all of the participants of the course team to that new assignment team.

If this is later confirmed to be the intended functionality of this method, then this method should belong to AssignmentTeam class instead of CourseTeam class.

This method calls the create_node_object method in Team class, which does not work (explained in a later section) and has an ambiguous return type, functionality, and name.

Logically, the copy method should create a new course team and copy the participants of a team into the new course team. Currently, this is dead code and not used in the program.

This function was marked deprecated and it is recommended to be removed from the program.

add_participant Method

This method is used to add a participant to the course that the course team to which it belongs.

This method is called by the add_member method in the Team class to add a participant to the course when a member is added to a course team.

This method does belong in the Course class and not the CourseTeam class. The Course class already has the method add_participant with similar functionality.

This function was marked deprecated and it is recommended that the function should be removed from the program and the add_member method in the Team class should call the add_participant method from Course class.

Changes for /app/models/course_node.rb

  • models/course_node.rb
    • The "get" method was improved.
      • Added "get_courses_managed_by_user" method to hold logic extracted from the "get"method.
      • Added "get_course_query_conditions" method to hold logic extracted from the "get" method.
      • Modified any checks on if the user is a teaching assistant to use polymorphism to make the determination.
      • Removed the initial conditional for setting the "conditions" value since the remaining conditions was overwriting the value regardless of how the initial conditional was evaluated.
      • Renaming the "get" method was considered; however, the name of the method is necessary for polymorphism reasons related to Assignment nodes. As such, a comment was added to explain why the method name should remain.
      • The "sortorder" parameter is given a default value of "ASC" for ascending. This allowed for a related conditional to be removed from the method.
      • The "sortvar" parameter is given a default value of "name" for sorting by course name. This allowed for a related conditional to be removed from the method.
    • Renaming the "get_children" method was considered; however, the name of the method is necessary for polymorphism reasons related to Assignment Nodes. As such, a comment was added to explain why the method name should remain.
    • The "get_survey_distribution_id" method was removed after finding it to be dead code.
  • models/ta.rb
    • Added "is_teaching_assistant?" method to override the inherited method from the User model. This supported usage of polymorphism to determine if the user is a teaching assistant.
  • models/user.rb
    • Added "is_teaching_assistant?" method to enable usage of polymorphism to determine if the user is a teaching assistant.

Methods Found That Are Not Currently Being Used

The following methods were not found to be used currently by the project, but have been left in the code for potential future usages of the functionality:

  • /app/models/course.rb
    • Course.get_participant(user_id)
  • /app/models/course_participant.rb
    • CourseParticipant.copy(assignment_id)
    • CourseParticipant.get_parent_name
  • /app/models/course_node.rb
    • CourseNode.get_modified_date

Other Observations and Suggestion for Future Work

Redundant Code in Team Related Classes

Classes: Team, CourseTeam, AssignmentTeam

Files: team.rb, course_team.rb, assignment_team.rb

The class CourseTeam and AssignmentTeam both inherit from the class Team. However, there are a lot of duplicated functionalities. In order to adhere to the "DRY" principle, the refactoring mentioned earlier for the CourseTeam should be applied to AssignmentTeam and duplicated methods should be extracted to the super-class Team.

Naming Inconsistency in Team Related Classes

Classes: Team, CourseTeam, AssignmentTeam

Files: team.rb, course_team.rb, assignment_team.rb

Participants in a team have many different names in these three classes. For example: member, user, participants. Common vocabulary should be established in order to eliminate confusion.

Bugs in Team Class

Class: Team

File: team.rb

Method: create_node_object

The create_node_object method in Team class does not work because the class method get_parent_model in both CourseTeam and Assignment team is an instance method and not a class method.

Based on the code of the object and the usage of this code, the method is supposed to create a new course or assignment team along with a new team node and then return the newly created team as output. This function should be deprecated and removed from the class. The functionality should be moved to the new method in the team controller if the functionality is found to be necessary.

Inelegance Code in Team Class

Class: Team

File: team.rb

Methods: randomize_all_by_parent, check_for_existing, create_node_object, copy_members

These methods uses Object.const_get() with an input string that dynamically locates the object that is associated with different team types (assignment or course); however, the uses of Object.const_get() are inconsistent throughout these methods.

It is recommended that instead of using query methods, such as get_participant_type and get_parent_model, in the assignment and course team to return a string containing the name of the object, these methods should be used to return the object itself. This will eliminate the need for Object.const_get().

List of New Unit Tests and Testcases

The following lists shows new unit files and/or new testcases:

  • test/unit/course_node_test.rb (new test file)
    • test_get_children_with_show_true_and_non_TA_and_ascending_order_by_name
    • test_get_children_with_show_true_and_non_TA_and_descending_order_by_name
    • test_get_children_with_show_true_and_TA_and_ascending_order_by_name
    • test_get_children_with_show_true_and_TA_and_descending_order_by_name
    • test_get_children_with_show_false_and_non_TA_and_ascending_order_by_name
    • test_get_children_with_show_false_and_non_TA_and_descending_order_by_name
    • test_get_children_with_show_false_and_TA_and_ascending_order_by_name
    • test_get_children_with_show_false_and_TA_and_descending_order_by_name
    • test_get_children_with_show_false_and_non_TA_and_nil_order_by_nil
    • test_get_name
    • test_get_directory
    • test_get_creation_date
    • test_get_modified_date
    • test_get_teams
    • test_get_parent_id
  • test/unit/course_test.rb (previously existing test file)
    • test_get_teams
    • test_get_path
    • test_get_participants
    • test_get_participant
    • test_add_participant
    • test_copy_participants
  • test/unit/course_participant_test.rb (previously existing test file)
    • test_get_course_string
    • test_import
    • test_get_path
    • test_export
    • test_get_export_field
  • test/unit/course_team_test.rb (new test file)
    • test_retrieval
    • test_import_participants
    • test_export_participants
    • test_instance_export
    • test_handle_duplicate
    • test_import
    • test_class_export

List of Modified Unit Tests

The following unit test files and/or testcases were modified:

  • test/unit/course_test.rb
    • The line "fixtures :courses" was changed to "fixtures :courses,:teams,:users,:participants,:assignments,:nodes,:tree_folders".

How to Run the Project

How to Run Expertiza

  • Navigate to the project root directory in terminal
  • run the script
ruby script/server -b 0.0.0.0 -p 3000

How to Run the Unit Tests

One can either

  • run the unit tests in RubyMine

or

  • navigate to the project root directory in terminal and run the unit tests via this command
ruby -Itest test/unit/TEST_FILENAME.rb