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

From Expertiza_Wiki
Jump to navigation Jump to search

Background

Motivation

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. Similarly, it had too many lines of code. Code climate identified the following issues:

   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 using the code for when assignment_form.save condition was satisfied.

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

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. These include multiple “if”/”unless” statements as well as loops that may or may not be executed. I 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

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.

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. [Rubocop style guide](https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md) 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

We ran rspec tests, and made sure all the existing tests were successful.


Future improvements

Team Information

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