CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

Background

Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. assignments_controller.rb is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

Tasks

The branch condition size for create method is too high

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. Code climate has identified following issues with the method:

   Assignment Branch Condition size for create is too high. [56.37/15]
   Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called handle_assignment_form_save from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in create method. Similarly, few more helper methods were extracted out of handle_assignment_form_save to make sure each method was only doing a single task.

The branch condition size for edit method is too high

  Assignment Branch Condition size for edit is too high. [29.58/15]

The branch condition size for copy method is too high

   Assignment Branch Condition size for copy is too high. [19.42/15]

The branch condition size for empty_rubrics_list method is too high

The current implementation of the empty_rubrics_list method has many different branches of the code that could be executed. Code Climate identified Assignment Branch Condition size to be too high.

   Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]

These include multiple if/unless statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for empty_rubrics_list.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named remove_existing_questionnaire.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named remove_invalid_questionnaires.

The branch condition size for update_feedback_assignment_form_attributes method is too high

This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using elsif statements.

The cognitive complexity of create method is too high

     Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

The cognitive complexity of empty_rubrics_list method is too high

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks. There is a .each block on the assignment questionnaires and there is a .reject block on the rubric list entries nested within it.

  Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

We sought to solve this issue by extracting the .reject block into a separate method named “remove_existing_questionnaire”.

A line in the handle_current_user_timezonepref_nil method is too long

The existing code had a line length of 180 characters.

 Line is too long. [180/160]

Rubocop style guide recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.

Testing

The assignments_controller.rb file has a testing file called the assignment_controller_sepc.rb. We ran rspec tests, and made sure all the existing tests passed with all our changes.

Team Information

• Dave Bell (dbell5)
• Steven Green (stgreen)
• Abhishek Upadhyaya Ghimire (aupadhy3)
• Mentor: Anuja Kulkarni