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

From Expertiza_Wiki
Jump to navigation Jump to search
Line 30: Line 30:


== Changes for models/course_node.rb ==
== Changes for models/course_node.rb ==
* models/course_node.rb
** The "get" method was improved.  
** The "get" method was improved.  
*** Renaming the "get" method was considered; however, the name of the method is necessary for polymorphism reasons related to Assignment nodesAs such, a comment was added to explain why the method name should remain.   
*** Added "get_courses_managed_by_user" method to hold logic extracted from the "get"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.
*** 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.   
*** 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 "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.
*** 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.


== Miscellaneous Changes for Quality Improvement and Readability ==
== Miscellaneous Changes for Quality Improvement and Readability ==

Revision as of 19:30, 10 March 2013

Write-up of This Topic.

E608. Refactoring and Testing - Course Related Classes

Classes Involved:

  • models/course.rb (76 lines)
  • models/course_participant.rb (94 lines)
  • models/course_team.rb (116 lines)
  • 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 models/course.rb

The following changes were made:

  • Renamed create_node method to create_course_node to accurately reflect the functionality of the method.
  • Added the line "has_many :assigments, :dependent => :destroy" to allow CourseTest.test_destroy unit test to pass.

Changes for models/course_participant.rb

Changes for models/course_team.rb

Changes for 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.
      • 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.

Miscellaneous Changes for Quality Improvement and Readability

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/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_create_course_node

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".